nuttxpr commented on PR #14183:
URL: https://github.com/apache/nuttx/pull/14183#issuecomment-2408563848

   [**\[Experimental Bot, please feedback 
here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   **Yes, but lacks details.**
   
   Here's a breakdown with suggestions for improvement:
   
   **Strengths:**
   
   * **Addresses a real issue:** Accessing freed memory (the file list) after a 
thread exits is a crash risk.
   * **Clear summary:**  You've concisely explained the problem and solution.
   * **Testing environment noted:** You've indicated testing on a simulator.
   
   **Areas for Improvement:**
   
   **Impact:** This section needs significant expansion.  Be specific about:
   
   * **Existing feature changed:**  Which part of the scheduler is modified?
   * **Impact on user:**  Likely minimal, but state this explicitly.  Are there 
edge cases where a user might have relied on the old (faulty) behavior?
   * **Impact on build:**  Unlikely, but state if there are any configuration 
option changes.
   * **Other impacts:**  All "NO" should have brief justifications (e.g., 
"Impact on hardware: NO - This is a software-only change within the scheduler").
   
   **Testing:**
   
   * **Insufficient detail:** "sim" is too vague.  Specify:
       * Simulator used (e.g., QEMU, Renode)
       * NuttX configuration (which board/arch were you simulating?)
   * **Missing logs:**  Provide snippets of logs demonstrating:
       * The crash BEFORE your change
       * The correct behavior AFTER your change 
   
   **Additional Considerations:**
   
   * **Code Style:**  Ensure your code adheres to NuttX's coding conventions.
   * **Commit Messages:**  Write clear and concise commit messages explaining 
each change.
   
   **Example for Testing:**
   
   ```
   Testing:
   
   * Build Host: Linux (Ubuntu 20.04), x86_64, GCC 9.4.0
   * Target: QEMU simulator, sim-arm:nsh configuration
   
   Testing logs before change:
   
   ```
   (repeatedly starting and stopping a thread that opens files)
   ...
   ASSERTION FAIL: filelist->f_next != NULL at 
.../sched/task/task_exithook.c:XXX 
   ```
   
   Testing logs after change:
   
   ```
   (same test case as above)
   ... system runs stably, no assertions ...
   ``` 
   


-- 
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]

Reply via email to