Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Driver test issue fix #2951

Merged
merged 3 commits into from
Jan 16, 2025
Merged

Conversation

Zhangshoukui
Copy link
Contributor

Summary

1.Fixed skipped logic errors
2.Fix the GPIO test sim error

Impact

driver test

Testing

./tools/configure.sh -l sim:citest
make -j20

nsh> cmocka_driver_gpio
[==========] tests: Running 1 test(s).
[ RUN ] drivertest_gpio
[input and output test] outvalue is 1, invalue is 1
[input and output test] outvalue is 0, invalue is 0
[input and output test] outvalue is 0, invalue is 0
[input and output test] outvalue is 0, invalue is 0
[input and output test] outvalue is 1, invalue is 1
[input and output test] outvalue is 0, invalue is 0
[input and output test] outvalue is 0, invalue is 0
[input and output test] outvalue is 0, invalue is 0
[input and output test] outvalue is 1, invalue is 1
[input and output test] outvalue is 1, invalue is 1
[ OK ] drivertest_gpio
[==========] tests: 1 test(s) run.
[ PASSED ] 1 test(s).

txy-21 and others added 3 commits January 15, 2025 15:47
Signed-off-by: tengshuangshuang <[email protected]>
…int to a stack space

==1805058==ERROR: AddressSanitizer: heap-use-after-free on address 0xe18126a0 at pc 0x52b06320 bp 0xd7b13ee8 sp 0xd7b13ed8
READ of size 1 at 0xe18126a0 thread T0
    #0 0x52b0631f in tre_parse regex/regcomp.c:1356
    apache#1 0x52b2b1d0 in regcomp regex/regcomp.c:3710
    apache#2 0x48f55435 in c_regexmatch cmocka/src/cmocka.c:494
    apache#3 0x48f65bcf in _cmocka_run_group_tests cmocka/src/cmocka.c:3252
    apache#4 0x48f67e2d in cmocka_fs_test_main apps/testing/testsuites/kernel/fs/cmocka_fs_test.c:201
    apache#5 0x46210b2a in nxtask_startup sched/task_startup.c:72
    apache#6 0x45ff40fb in nxtask_start task/task_start.c:116
    apache#7 0x462695bb in pre_start sim/sim_initialstate.c:52

Signed-off-by: zhangshoukui <[email protected]>
cmocka_driver_gpio //gpio input/output is tested by default
cmocka_driver_gpio -a /dev/gpio0 -b /dev/gpio1 -l // test loop
cmocka_driver_gpio -a /dev/gpio0 -b /dev/gpio0 // gpio input/output is tested by default

Signed-off-by: zhangshoukui <[email protected]>
@nuttxpr
Copy link

nuttxpr commented Jan 16, 2025

[Experimental Bot, please feedback here]

This PR does not fully meet the NuttX requirements. While it provides some information, it lacks crucial details.

Here's a breakdown:

  • Summary: While it mentions what was fixed ("skipped logic errors" and a "GPIO test sim error"), it doesn't explain why the errors occurred or how the fixes address the underlying issues. It needs more detail. Also, it should link a related NuttX issue if one exists.

  • Impact: Simply stating "driver test" is insufficient. It needs to explicitly address all the listed impact points (user, build, hardware, documentation, security, compatibility) with "YES" or "NO" and provide descriptions where necessary. Even if the answer is "NO" for most, it should be stated explicitly.

  • Testing: While it shows how the test was run, it doesn't provide "before" and "after" logs as requested. The provided log is only the "after" result. It also lacks details about the build host. Just stating "sim:citest" is not enough; specify the host OS, compiler version, etc. It needs logs demonstrating the failure before the change and the success after the change to prove the fix is effective.

Example of how to improve the PR description:

Summary

This PR fixes two issues related to the GPIO driver test in the simulator:

  1. Skipped Logic Error: The original test code inadvertently skipped a critical logic check for GPIO input/output functionality due to [explain the reason for the skipped logic, e.g., an incorrect conditional statement]. This resulted in incomplete testing and could have masked potential bugs. This fix corrects the conditional logic to ensure all code paths are tested.

  2. GPIO Test Sim Error: The GPIO simulator driver produced an error [explain the specific error encountered, e.g., an incorrect return value or segmentation fault] during the test execution due to [explain the root cause, e.g., an uninitialized variable]. This fix [explain how the fix addresses the root cause, e.g., by initializing the variable].

Related NuttX Issue: [If applicable, link to the related issue]

Impact

  • Is new feature added? NO
  • Is existing feature changed? YES (The GPIO driver test is corrected, improving test coverage and reliability).
  • Impact on user: NO
  • Impact on build: NO
  • Impact on hardware: NO
  • Impact on documentation: NO
  • Impact on security: NO
  • Impact on compatibility: NO
  • Anything else to consider: NO

Testing

I confirm that changes are verified on local setup and works as intended:

  • Build Host(s): Linux (Ubuntu 22.04), x86_64, GCC 11.3.0
  • Target(s): sim:citest

Testing logs before change:

[==========] tests: Running 1 test(s).
[ RUN      ] drivertest_gpio
[input and output test] Segmentation fault
[  FAILED  ] drivertest_gpio
[==========] tests: 1 test(s) run.
[  FAILED  ] 1 test(s).

Testing logs after change:

[==========] tests: Running 1 test(s).
[ RUN      ] drivertest_gpio
[input and output test]  outvalue is 1, invalue is 1
[input and output test]  outvalue is 0, invalue is 0
... (rest of the successful log) ...
[  PASSED  ] 1 test(s).

By providing more specific information, the PR becomes much easier to review and understand, increasing its chances of being accepted.

Copy link

@cederom cederom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Zhangshoukui :-)

@xiaoxiang781216 xiaoxiang781216 merged commit f8e4afb into apache:master Jan 16, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants