nuttxpr commented on PR #2946: URL: https://github.com/apache/nuttx-apps/pull/2946#issuecomment-2591785220
[**\[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 fix it: **Missing Information in Summary:** * **Why is the change necessary?** Simply stating "Saving stack" isn't enough. Explain the problem the change solves. Is there a stack overflow issue? Is this a preventative measure for potential future issues? Quantify the savings if possible. * **What functional part of the code is being changed?** Be specific. Which files/functions are using `lib_get_tempbuffer` now that weren't before? * **How does the change exactly work?** Explain the mechanism. Does it replace statically allocated buffers? How is memory managed? **Missing Information in Impact:** * **Is new feature added? Is existing feature changed?** Answer these explicitly. * While you mention saving stack space, you need to answer all the other impact questions (build, hardware, documentation, security, compatibility) with explicit YES/NO and descriptions where applicable. Even if the answer is NO, state it explicitly. **Missing Information in Testing:** * **Build Host(s):** Provide details about your build host environment. e.g., "Linux, x86_64, GCC 12.1" * **Target(s):** Be more specific than "sim:nsh." Which simulator? Which configuration within nsh? * **Testing logs before change:** You've provided no logs. Show the output demonstrating the problem *before* your change. For example, perhaps there's a stack overflow error message or you've used a tool to measure stack usage. * **Testing logs after change:** You've provided no logs. Show the output *after* your change demonstrating the improvement. Again, an error message disappearing or reduced stack usage metrics would be helpful. Show the output of the same test commands you used *before* the change to allow for direct comparison. **Example of an improved PR description:** ## Summary This change addresses potential stack overflow issues by utilizing `lib_get_tempbuffer` for temporary buffers in the `xyz` and `abc` functions within the `drivers/networking/foo` directory. Previously, these functions used large, statically allocated buffers on the stack, consuming up to `N` bytes. This change moves the buffer allocation to the heap, reducing stack usage and improving system stability. This addresses [NuttX Issue #XXX](https://github.com/apache/nuttx/issues/XXX) (if applicable). ## Impact * Is new feature added? NO * Is existing feature changed? YES. The memory management strategy for temporary buffers in the `foo` driver is changed. * Impact on user: NO * Impact on build: NO * Impact on hardware: NO * Impact on documentation: YES. The `drivers/networking/foo/README.md` file has been updated to reflect the new memory management approach. * Impact on security: NO * Impact on compatibility: NO * Anything else to consider? This change might slightly increase heap memory usage. ## Testing I confirm that changes are verified on local setup and works as intended: * Build Host(s): Linux, x86_64, GCC 12.1 * Target(s): sim:qemu-x86_64, nsh configuration `xyz` Testing logs before change: ``` $ ./my_stack_test_app Stack usage: 95% ERROR: Stack overflow detected! ``` Testing logs after change: ``` $ ./my_stack_test_app Stack usage: 20% Test passed. ``` -- 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
