You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "wyr-7 (via GitHub)" <gi...@apache.org> on 2024/10/11 14:05:12 UTC
[PR] testing/drivertest_block.c: Fix cmoka driver block test error [nuttx-apps]
wyr-7 opened a new pull request, #2693:
URL: https://github.com/apache/nuttx-apps/pull/2693
## Summary
testing/drivertest_block.c: Fix cmoka driver block test error
## Impact
1. Fix the problem that pre->source may have memory stampede
2. Fix total block size larger than cache size condition
## Testing
Test in Vela
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] testing/drivertest_block.c: Fix cmoka driver block test error [nuttx-apps]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #2693:
URL: https://github.com/apache/nuttx-apps/pull/2693#issuecomment-2407516968
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
This pull request appears to **partially meet** the NuttX requirements. Here's a breakdown:
**Strengths:**
* **Summary:** Provides a clear and concise description of the issue addressed (memory stampede, block size exceeding cache size) and the file affected (`testing/drivertest_block.c`).
* **Impact:** Clearly outlines the functional impact of the changes (fixing errors).
* **Testing:** Indicates the testing environment (Vela), but lacks specific details.
**Areas for improvement:**
* **Summary:**
* Lacks a related issue reference if this PR addresses a known bug.
* **Impact:**
* The impact section is very brief. While it mentions fixes, it lacks details about potential user impact, build process changes, hardware dependencies, or documentation requirements.
* All other impact points are not addressed. While they may not be relevant, explicitly stating "NO" with a brief justification (e.g., "No impact on build process as changes are isolated to testing code") would provide a more thorough assessment.
* **Testing:**
* **Crucially**, it lacks testing logs before and after the change. These logs are essential to demonstrate the actual issue and the effectiveness of the fix.
**Recommendation:**
To fully meet the NuttX requirements, the PR should include:
1. **Related Issue:** If applicable, link the corresponding NuttX issue that this PR addresses.
2. **Detailed Impact Assessment:** Address all impact points, even if the answer is "NO." Briefly explain why there is no impact for each point.
3. **Complete Testing Information:** Provide the complete testing logs **before** and **after** the change to clearly demonstrate the fix. This includes:
* Build host details: OS, CPU architecture, compiler version.
* Target details: Architecture (e.g., sim, RISC-V, ARM), board configuration.
* Relevant log snippets showing the error before the fix and the corrected behavior after.
By addressing these points, the PR will be more comprehensive and easier for reviewers to assess.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] testing/drivertest_block.c: Fix cmoka driver block test error [nuttx-apps]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #2693:
URL: https://github.com/apache/nuttx-apps/pull/2693
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] testing/drivertest_block.c: Fix cmoka driver block test error [nuttx-apps]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #2693:
URL: https://github.com/apache/nuttx-apps/pull/2693#discussion_r1799958073
##########
testing/drivertest/drivertest_block.c:
##########
@@ -326,9 +326,16 @@ static void blktest_cachesize_write(FAR void **state)
size = block * pre->geo.blocksize;
+ if (size > pre->geo.erasesize * pre->geo.neraseblocks)
+ {
+ printf("Warning: Total block size too small, need larger than %ld\n",
Review Comment:
```suggestion
printf("Warning: Total block size too small, need larger than %" PRId32 "\n",
```
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org