apps/nshlib: Using lib_get_tempbuffer() to save stack space

[JianyuWang0623]

Summary

Using lib_get_tempbuffer() to save stack space

1. apps/nshlib: Using lib_get_tempbuffer() to save stack space
2. apps/nshlib: replace CONFIG_NSH_LINELEN to LINE_MAX
3. apps/system: replace CONFIG_NSH_LINELEN to LINE_MAX (Picked from apps/system: replace CONFIG_NSH_LINELEN to LINE_MAX #2918 by @anchao )

Impact

• apps/nshlib
• apps/system

Testing

1. Selftest as commit message shown

   • Config: "esp32s3-devkit:adb" with CONFIG_LINE_MAX=512

   • Test CMD: ls | cat | cat | cat

   • Without this patch

     	Before Test CMD	After Test CMD
     nsh_main.STACK	0002624/0008096	0002624/0008096
     sh.STACK	0003360/0004008	0003360/0004008
     Free/Total	355288/403116	355288/403116

   • With this patch

     	Before Test CMD	After Test CMD
     nsh_main.STACK	0001616/0008096	0001616/0008096
     sh.STACK	0002352/0004008	0002352/0004008
     Free/Total	355288/403116	354760/403116

2. CI

[nuttxpr]

[Experimental Bot, please feedback here]

This PR description mostly meets the NuttX requirements but could be improved. Here's a breakdown:

Strengths:

• Clear Summary of Changes: The summary clearly states what was changed and why. The use of bullet points and references to related issues/PRs is good.
• Impact Section Started: Addressing the impact is started, but needs more detail.
• Testing Information Provided: The testing section includes the host, target, config, commands used, and results. This is excellent.

Weaknesses:

• Summary Lacks "How": While the what is clear, the summary doesn't fully explain how lib_get_tempbuffer() saves stack space. Explain the mechanism briefly (e.g., it allocates buffers from the heap instead of the stack).
• Incomplete Impact Assessment: The Impact section is too brief. It lists affected components but doesn't fully address all the required points. Every "YES" needs a description. For each "NO", explicitly state "NO". Example for "Impact on user": "YES, users of NSH will see slightly reduced heap space availability but improved stack usage." (Even if seemingly minor, describe it).
• Missing "Before" and "After" Logs in Testing Section: The tables show the stack usage before and after the test command, but the actual console output/logs from the tests are missing.
• Unclear CI Status: Mentioning "CI" is insufficient. State whether CI passed or provide a link to the CI run.

Suggested Improvements:

Summary

• Why: NSH commands often use large buffers on the stack, potentially leading to stack overflows, especially with deeply nested pipelines. Using lib_get_tempbuffer() allocates these buffers from the heap, reducing stack usage and mitigating this risk.
• What: This change modifies the NSH library and related applications to use lib_get_tempbuffer() for buffer allocation. It also replaces CONFIG_NSH_LINELEN with LINE_MAX for consistency.
• How: Instead of declaring large character arrays on the stack, lib_get_tempbuffer() is used to dynamically allocate memory from the heap for these buffers. This memory is then freed when no longer needed.
• Related Issues/PRs: (List any related issues/PRs)
• apps/nshlib: Using lib_get_tempbuffer() to save stack space
• **apps/nshlib: replace CONFIG_NSH_LINELEN to LINE_MAX
• **apps/system: replace CONFIG_NSH_LINELEN to LINE_MAX (Picked from apps/system: replace CONFIG_NSH_LINELEN to LINE_MAX #2918 by @anchao )

Impact

• New Feature Added: NO
• Existing Feature Changed: YES, NSH buffer management.
• Impact on User: YES, users of NSH will see slightly reduced heap space availability due to dynamic buffer allocation, but significantly improved stack safety, reducing the risk of stack overflows.
• Impact on Build: NO
• Impact on Hardware: NO
• Impact on Documentation: YES, documentation should be updated to reflect the use of lib_get_tempbuffer() and the change from CONFIG_NSH_LINELEN to LINE_MAX.
• Impact on Security: YES, This change improves security by reducing the risk of stack overflow vulnerabilities.
• Impact on Compatibility: NO
• Anything else to consider: None

Testing

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

• Build Host(s): OS: Linux, CPU: Intel, Compiler: GCC (Provide version)
• Target(s): arch: sim (esp32s3-devkit:adb), board: esp32s3-devkit, config: adb, CONFIG_LINE_MAX=512

Testing logs before change:

# Actual command output and any relevant messages here, not just stack usage.
ls
...output...
cat | cat | cat
...output...
free
...output... # Showing heap usage before

Testing logs after change:

# Actual command output and any relevant messages here
ls
...output...
cat | cat | cat
...output...
free
...output... # Showing heap usage after

Stack Usage Comparison:

(Include your tables as before)

CI Status: [Link to CI run] or "CI passed successfully."

By making these improvements, the PR description will be clearer, more complete, and better aligned with the NuttX requirements. This will make it easier for reviewers to understand and approve the changes.

[JianyuWang0623]

@xiaoxiang781216 @anchao Could you review this PR please?

[JianyuWang0623]

TODO: Remove NSH_LINELEN from Kconfig after this PR was merged

1. Replace NSH_LINELEN with LINE_MAX for nuttx/configs -- boards: Replace CONFIG_NSH_LINELEN with CONFIG_LINE_MAX nuttx#15541
2. Remove NSH_LINELEN from apps/nshlib/Kconfig -- apps/nshlib: Remove the deprecated config NSH_LINELEN #2945

[cederom]

Thank you @JianyuWang0623 amazing work squeezing every bit and byte out of hw :-)

Lets just wait for the CI to complete test builds :-)