nuttxpr commented on PR #13988: URL: https://github.com/apache/nuttx/pull/13988#issuecomment-2401995062
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This Pull Request summary does **not** meet the NuttX requirements. Here's why and how to improve it: **Issues:** * **Summary Lacks Detail:** * **Why is the change necessary?** Is it fixing a bug, adding a missing standard feature, or an optimization? Be specific about the problem this PR solves. * **What part of the code is changed?** Mention the specific files/modules affected (e.g., "Added setjmp/longjmp implementation to the libc directory"). * **How does it work?** Briefly explain the implementation approach (e.g., "Implemented using processor-specific context saving routines"). * **Impact is Too Vague:** * **"No impact" is rarely completely true.** Even adding standard functions can have subtle impacts: * **New Feature:** Clearly state that this adds setjmp/longjmp functionality. * **Build System:** Did you add any new configuration options related to enabling/disabling setjmp/longjmp? * **Documentation:** Does this require updates to the NuttX documentation? * **Testing is Insufficient:** * **Target Specificity:** * **Architecture:** Be specific about which architectures you tested on (e.g., "Tested on ARM Cortex-M4 simulator and RISC-V QEMU"). * **Boards/Configurations:** List the exact board configurations if applicable. * **"ostest" is Not Enough:** Provide more descriptive test cases: * **Demonstrate the problem before the change:** Include logs or outputs showing the issue this PR addresses. * **Show successful execution after the change:** Include logs showing how setjmp/longjmp now function correctly. **Improved PR Summary Example** ```markdown ## Summary This PR adds the standard `setjmp()` and `longjmp()` functions to the NuttX libc, providing support for non-local jumps within a program. These functions were previously unavailable, limiting certain programming paradigms. The implementation is located in `libc/stdlib/lib_setjmp.c` and leverages architecture-specific context saving mechanisms provided by the compiler's `setjmp.h`. ## Impact * **New Feature:** Adds `setjmp()` and `longjmp()` functionality to the C standard library. * **Build System:** No changes to the build system are required. * **Documentation:** Updates to the libc documentation are included in this PR to reflect the addition of `setjmp()` and `longjmp()`. ## Testing I confirm that changes are verified on the following setups: * **Build Host:** Linux (Ubuntu 20.04), GCC 9.3.0 * **Target 1:** ARM Cortex-M4 simulator (QEMU) * **Target 2:** RISC-V (QEMU) **Testing Before Change (ARM Cortex-M4):** ``` <Show output demonstrating the lack of setjmp/longjmp support or the specific problem being solved> ``` **Testing After Change (ARM Cortex-M4):** ``` <Show output of a test program successfully using setjmp() and longjmp()> ``` **Testing After Change (RISC-V):** ``` <Show output of a test program successfully using setjmp() and longjmp()> ``` ``` **Remember:** A good PR summary is clear, concise, and provides all the information reviewers need to understand and evaluate your changes. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
