nuttxpr commented on PR #14877: URL: https://github.com/apache/nuttx/pull/14877#issuecomment-2488335566
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR description, as written, does *not* fully meet the NuttX requirements. Here's why and how to fix it: **Missing Information in Summary:** * **Why is this change necessary?** The summary states *what* it does but not *why*. Is this for testing purposes? To improve debugging capabilities? To support systems without filesystems for code coverage? Explain the motivation. * **What functional part of the code is changed?** Be specific. Which files/modules are affected? Is this a change to the core OS, a specific driver, a build system modification? * **How does the change exactly work?** "Execute the app that calls __llvm_profile_dump" is vague. Explain the mechanism. Does it use a dedicated memory region? Does it hook into existing functionality? How is the data retrieved if not using a filesystem? **Incomplete Impact Assessment:** The impact section is essentially empty. Address *all* the points: * **New/Changed Feature:** Yes, new feature (data storage for code coverage without a filesystem). Describe the feature briefly. * **Impact on User:** Potentially YES. How will users enable/use this feature? Are there any configuration options? If the answer is NO, explicitly state it and justify why. * **Impact on Build:** Possibly YES if compiler flags need to be set. Describe how the build process is affected. If NO, explicitly state so. * **Impact on Hardware:** Probably NO. If so, state NO. If specific architectures are affected, explain how. * **Impact on Documentation:** YES. Documentation is required to explain how to use this new feature. State that documentation is included in the PR or will be provided in a follow-up. * **Impact on Security:** Potentially YES or NO. Does this change introduce any security risks (e.g., memory corruption)? Does it improve security by aiding in vulnerability discovery? Justify your answer. * **Impact on Compatibility:** Potentially YES or NO. Does this break existing code? Does it affect compatibility with different LLVM versions? Explain. * **Anything else to consider:** Are there any performance implications? Resource usage considerations? **Insufficient Testing Information:** * **Build Host(s):** Provide details about the build host operating system, CPU architecture, and compiler version used for testing. * **Target(s):** Be specific about the target architecture, board, and configuration. "qemu-system-arm -M mps3-an547 -nographic" is a good start, but mention the NuttX configuration used (e.g., `gcov`). * **Testing logs before change:** Include actual logs or output demonstrating the behavior before the change. What happens when you try to get coverage data without a filesystem *before* this PR? Likely an error. Show that. * **Testing logs after change:** Show logs demonstrating the successful retrieval of coverage data using the new method. Include example output from the `llvm-cov` tools to show that it works. **Example of an Improved Summary:** This PR introduces support for collecting code coverage data without relying on a file system. This is essential for environments where a file system is unavailable or undesirable, such as deeply embedded systems or during early boot stages. The change modifies the `arch/arm/src/xxx` and `libs/xxx` modules to store coverage data in a dedicated memory region. The data is then retrieved via a new system call or by examining memory directly using a debugger. This feature is enabled by setting `CONFIG_XXX` and requires compiling with `-fprofile-instr-generate -fcoverage-mapping`. This addresses issue #[NuttX Issue Number]. By providing more complete and specific information in each section, the PR will be much easier to review and understand, increasing the likelihood of it being accepted. Remember, clarity and detail are key to a good PR. -- 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
