nuttxpr commented on PR #15380: URL: https://github.com/apache/nuttx/pull/15380#issuecomment-2564948663
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not fully meet the NuttX requirements. Here's why and how to improve it: **Missing Information in Summary:** * **What functional part of the code is being changed?** Be specific. e.g., "The TCP/IP stack, specifically the `accept()` function implementation." * **How does the change exactly work?** Explain the code changes you made to prevent the listening socket from being notified. For example, "The change prevents propagation of the TCP_RESET event from the accepted socket to the listening socket by [explain the mechanism, e.g., modifying a flag, changing an if condition, etc.]." **Missing Information in Impact:** Fill out *all* impact sections. Even if the answer is "NO," explicitly state it. This PR likely has implications for: * **Is new feature added? Is existing feature changed?** This is a bug fix, so "Existing feature changed" should be marked. Briefly describe the original faulty behavior. * **Impact on user:** Potentially YES. Explain that the incorrect POLLERR/POLLHUP behavior is fixed, which might affect applications relying on (or misinterpreting) the previous behavior. Explain how users who *were* relying on the bug should adapt. * **Impact on documentation:** If the previous behavior was documented (even implicitly), "YES" is needed. Briefly explain what documentation needs updating. If no documentation changes are needed, explicitly state "NO - The change corrects undocumented, erroneous behavior." * **Impact on compatibility:** Potentially YES if applications were relying on the bug. State "Potentially YES. Applications incorrectly relying on the previous POLLERR/POLLHUP behavior on the listening socket after a TCP_RESET on the accepted socket will need to be adjusted." If you're confident there's no compatibility impact, explain why. **Missing Information in Testing:** * **Build Host(s):** Be specific. Example: "Linux Ubuntu 20.04, x86_64, GCC 9.4.0" * **Target(s):** Be *very* specific. Example: "STM32F746NGH6 (Nucleo-F746ZG board), NuttX configuration: `nsh`" * **Testing logs before change:** Include actual logs demonstrating the problem. Show that the listening socket receives POLLERR/POLLHUP when a client connection resets. * **Testing logs after change:** Include logs demonstrating that the listening socket *no longer* receives POLLERR/POLLHUP under the same reset conditions. Show that the accepted socket handles the reset gracefully. **Example of an Improved Summary:** The TCP/IP stack, specifically the `accept()` function implementation, is changed to correct the handling of TCP_RESET events. Previously, when a client connection reset after being accepted, the listening socket was also notified of the event, resulting in POLLERR and POLLHUP being returned by `poll()`. This change isolates the TCP_RESET event to the accepted socket, preventing this incorrect notification to the listening socket. This fixes unexpected behavior that could lead to incorrect application logic. Fixes [NuttX Issue # if applicable]. By providing this more complete information, your PR will be much easier to review and understand, increasing its chances of being merged quickly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
