nshlib/cmd_wait: Wait failed if FS_PROCFS not enabled

[JianyuWang0623]

Summary

Wait failed if FS_PROCFS not enabled

Env

# sim:nsh
- CONFIG_FS_PROCFS=y

Error

nsh> sleep 5 &
sh [4:100]
nsh> wait 4
nsh: wait: wait failed: 2

cmd_wait()

snprintf(path, sizeof(path), "/proc/%d/status", tid);

Reported by @XuNeo and @Gary-Hobson, thank you;-)

Impact

nshlib/cmd_wait

Testing

1. Selftest: Using sim:nsh with CONFIG_FS_PROCFS disabled - PASSED
2. NuttX CI

[nuttxpr]

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although the provided information is minimal.

Here's a breakdown of why and what could be improved:

Strengths:

• Clear Summary: The summary concisely explains the bug, the affected code, and the general mechanism of the fix (accessing a procfs entry that doesn't exist when procfs is disabled).
• Impact is identified: The impact on nshlib/cmd_wait is noted.
• Testing is described: The testing methodology is explained, referencing both local testing and CI.

Weaknesses & Areas for Improvement:

• Missing "Why" in Summary: While the bug is described, the PR doesn't explicitly state why this change is necessary. Is wait expected to work without procfs? Should it return a different error code? This needs clarification.
• How it works is vague: "accessing a procfs entry" isn't sufficient. Did you add a check for CONFIG_FS_PROCFS? Did you change the error handling? The PR description should clearly explain the code changes.
• Impact is incomplete: While nshlib/cmd_wait is mentioned, the explicit YES/NO answers for each impact category are missing. This makes it harder to quickly assess the scope of the change. For example:
  • Impact on user: YES (users relying on wait without procfs will see different behavior). Describe the new behavior.
  • Impact on build: Probably NO (unless you added build dependencies).
  • Impact on hardware: NO
  • Impact on documentation: Potentially YES if the behavior of wait is now different and needs documenting.
  • Impact on security: Likely NO.
  • Impact on compatibility: Potentially YES. Explain if the change breaks any existing scripts or usage patterns.
• Testing logs are missing: The template clearly requests "Testing logs before change" and "Testing logs after change". These are crucial for reviewers to understand the impact and verify the fix. Include the actual output of the wait command both before and after your changes.
• Build Host(s)/Target(s) incomplete: Provide more details about your local testing environment. "sim:nsh" is a start, but specify the architecture (e.g., sim:qemu-x86_64).

In short, while the core information is present, this PR description needs more detail and completeness to meet the NuttX standards fully. Filling in the missing information will significantly improve the review process and the overall quality of the contribution.

[cederom]

Thank you @JianyuWang0623 :-)

[xiaoxiang781216]

Please remove CONFIG_NSH_DISABLE_WAIT from boards/arm/mps/mps3-an547/configs/ap/defconfig:

====================================================================================
Configuration/Tool: mps3-an547/ap,CONFIG_ARM_TOOLCHAIN_GNU_EABI
2024-11-13 13:37:27
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
  Disabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
  Enabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
  Building NuttX...
  Normalize mps3-an547/ap
68d67
< # CONFIG_NSH_DISABLE_WAIT is not set
Saving the new configuration file
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   boards/arm/mps/mps3-an547/configs/ap/defconfig

[JianyuWang0623]

Please remove CONFIG_NSH_DISABLE_WAIT from boards/arm/mps/mps3-an547/configs/ap/defconfig:

====================================================================================
Configuration/Tool: mps3-an547/ap,CONFIG_ARM_TOOLCHAIN_GNU_EABI
2024-11-13 13:37:27
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
  Disabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
  Enabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
  Building NuttX...
  Normalize mps3-an547/ap
68d67
< # CONFIG_NSH_DISABLE_WAIT is not set
Saving the new configuration file
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   boards/arm/mps/mps3-an547/configs/ap/defconfig

Get, done, thank you, please review: apache/nuttx#14776
And a simply check by execuating find boards/ -name defconfig | while read i; do if grep CONFIG_NSH_DISABLE_WAIT $i >&/dev/null; then if ! grep CONFIG_FS_PROCFS $i >&/dev/null; then echo $i; fi; fi; done

[JianyuWang0623]

It seems that the CI not running with the latest apache/master even if it was triggered after apache/nuttx#14776 was merged.
Local test OK

$ git checkout apache/master 
Previous HEAD position was 6554ed4d668 get g_tcbinfo just use elf memory
HEAD is now at 85eed31443e mps3-an547:ap: Disable cmd_wait() as it depends on procfs currently
$ yes | ./tools/refresh.sh --defaults mps3-an547:ap
  Normalize mps3-an547:ap
$

[xiaoxiang781216]

try git push -f ... again.