Skip to content

Conversation

@lumapu
Copy link

@lumapu lumapu commented Jul 7, 2025

The value is checked here to be different from zero:

if (_onDisconnectfn) {
_onDisconnectfn();
}

To have a defined behavior I would recommend to set _onDisconnectfn to NULL

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@mathieucarbou mathieucarbou requested a review from Copilot July 7, 2025 19:24

This comment was marked as outdated.

@mathieucarbou mathieucarbou requested a review from Copilot July 8, 2025 08:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR initializes the newly added _onDisconnectfn member to NULL in the AsyncWebServerRequest constructor, preventing undefined behavior when the pointer is later checked.

  • Added _onDisconnectfn(NULL) to the initializer list.
Comments suppressed due to low confidence (2)

src/WebRequest.cpp:25

  • [nitpick] Consider using nullptr instead of NULL for pointer initialization in C++ to improve type safety and clarity.
  : _client(c), _server(s), _handler(NULL), _response(NULL), _onDisconnectfn(NULL), _temp(), _parseState(PARSE_REQ_START), _version(0), _method(HTTP_ANY),

src/WebRequest.cpp:25

  • [nitpick] Ensure the initializer list order matches the declaration order of class members to avoid compiler warnings and improve maintainability.
  : _client(c), _server(s), _handler(NULL), _response(NULL), _onDisconnectfn(NULL), _temp(), _parseState(PARSE_REQ_START), _version(0), _method(HTTP_ANY),

@mathieucarbou mathieucarbou merged commit 049a659 into ESP32Async:main Jul 8, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants