nuttxpr commented on PR #16163: URL: https://github.com/apache/nuttx/pull/16163#issuecomment-2789005943
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR summary does not fully meet the NuttX requirements. While it addresses the core issue and impact, it lacks crucial details needed for a proper review. Here's a breakdown of what's missing: * **Summary:** * Missing a clear explanation of *how* the linker scripts are unified. What specific changes were made? What was the root cause of the unaligned access? * Missing links to related NuttX Issues (if any). * **Impact:** While it mentions a fix and linker script unification, it needs to be more explicit using the YES/NO format and providing descriptions where applicable: * **Is new feature added?** NO * **Is existing feature changed?** YES (Linker scripts for stm32f0l0g0 are now unified). * **Impact on user:** NO (Unless they were relying on the previous, broken behavior). It's worth mentioning explicitly if the fix impacts application code in any way. * **Impact on build:** YES (Linker scripts changed). Specify *which* linker scripts. * **Impact on hardware:** YES (stm32f0l0g0). Be more specific about which boards are affected. * **Impact on documentation:** Maybe? Does this require documentation updates? Even if NO, explicitly state it. * **Impact on security:** NO (unless the unaligned access had security implications, which is unlikely but should be considered). * **Impact on compatibility:** NO (unless there are known compatibility issues). * **Testing:** * Insufficient detail. Provide the specific OS, CPU, compiler used on the build host. * Be explicit about the target architecture (e.g., armv6-m) and board configuration used (e.g., `nucleo-c071rb:nsh`). * The "Testing logs" sections are empty. Provide *actual* logs demonstrating the issue before the change and the corrected behavior after the change. Show evidence of the crash before and its absence after. **Example of improved summary:** ``` ## Summary This PR unifies the linker scripts for all stm32f0l0g0 boards by [explain the specific changes, e.g., moving rodata to a specific section, aligning it differently, etc.]. This addresses a crash caused by unaligned access to rodata during initialization on these boards. Previously, [explain the previous behavior and why it led to unaligned access]. Related Issue: [Link to NuttX issue if applicable] ## Impact * Is new feature added? NO * Is existing feature changed? YES (Linker scripts for stm32f0l0g0, specifically `script1.ld`, `script2.ld`, and `script3.ld`, are now unified.) * Impact on user: NO (This fix corrects a crash, so users should see improved stability. Applications built against the previous, incorrect linker scripts might need to be rebuilt). * Impact on build: YES (The linker scripts `script1.ld`, `script2.ld`, and `script3.ld` are modified). * Impact on hardware: YES (stm32f0l0g0 family, affecting boards nucleo-c071rb and [list other affected boards]). * Impact on documentation: NO * Impact on security: NO * Impact on compatibility: NO * Anything else to consider: None ## Testing I confirm that changes are verified on local setup and works as intended: * Build Host(s): Linux (Ubuntu 20.04), x86_64, GCC 9.4.0 * Target(s): armv6-m, nucleo-c071rb:nsh Testing logs before change: ``` [show log demonstrating the crash, e.g., hard fault handler, memory access violation, etc.] ``` Testing logs after change: ``` [show log demonstrating the correct behavior after applying the patch, e.g., successful initialization, application running as expected] ``` ``` By providing these details, reviewers can understand the change, assess its impact, and verify its correctness more efficiently. This increases the chances of the PR being accepted. -- 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
