nshlib/nsh_session: Arguments of subcommand was lost

[JianyuWang0623]

Summary

Fix a bug that arguments of subcommand will be lost.

1. It`s fine if subcommand has no argument sh -c dd

sh -c dd

2. Argument(s) of subcommand will be lost sh -c dd bs=1

sh -c dd bs=1

# Merge to one argument by using quotation is OK
sh -c "dd bs=1"

3. Error case

nsh> ls | dd bs=1 | cat

 argv[0] = "sh";
 argv[1] = "-c";
 argv[2] = "dd";
 argv[3] = "bs=1";

 nsh_execute(..., argv, ...);

Impact

Bugfix of nshlib/nsh_session

Testing

1. Selftest as commit message show.
2. NuttX CI

[nuttxpr]

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the basic NuttX requirements, but could be improved. Here's why and how:

Strengths:

• Addresses a clear bug: The summary explains the problem and provides examples.
• Specifies the affected code: nshlib/nsh_session is identified.
• Includes testing information: Selftest and NuttX CI are mentioned.

Weaknesses and Suggestions for Improvement:

• Summary Lacks Detail: While the problem is stated, the how is missing. How was the bug fixed? What was the root cause? Be more specific about the code changes implemented. For example, "The bug was caused by incorrect parsing of subcommand arguments within the parse_command() function. The fix involves adding a check for subcommand arguments and properly forwarding them to the executed command."
• Impact Section Lacks Detail: While stating "Bugfix of nshlib/nsh_session" is a start, the impact section requires YES/NO answers for all items. Even if the answer is NO, explicitly state it. For example:
  • Impact on user: YES (Users will no longer experience the loss of subcommand arguments when using sh -c)
  • Impact on build: NO
  • Impact on hardware: NO
  • Impact on documentation: NO (or YES if documentation needs updating to reflect the change)
  • Impact on security: NO (or YES if the bug had security implications)
  • Impact on compatibility: NO (Hopefully. State this explicitly. If there's any chance of incompatibility, explain it).
  • Anything else to consider? NO
• Testing Logs Missing: The template clearly states to include testing logs before and after the change. These are essential for demonstrating that the fix works and hasn't introduced regressions. Include actual log output, even if it's simple. Don't just say "Selftest as commit message show." Provide the relevant parts of the selftest output here. The CI results will be available separately, but including local testing logs is crucial. Show commands used and their output.
• Build Host Information Missing: Specify the build host environment. This helps reproduce the build and test results.

Example of Improved Testing Section:

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:qemu-x86_64


Testing logs before change:

nsh> sh -c "dd bs=1 count=10" < /dev/zero > /dev/null

Output omits 'bs=1 count=10', effectively running just 'dd' with default parameters

1+0 records in
1+0 records out
1 byte copied, 0.0000124 s, 80.7 kB/s

Testing logs after change:

nsh> sh -c "dd bs=1 count=10" < /dev/zero > /dev/null
10+0 records in
10+0 records out
10 bytes copied, 0.0000121 s, 826 kB/s

By addressing these weaknesses, the PR will be significantly stronger and easier for reviewers to evaluate. Remember, clarity and detail are key to a successful pull request.