nsh: input redirects #2469

casaroli · 2024-08-06T22:02:17Z

Summary

This is an attempt to refactor nsh to allow commands to have input redirects (<). It allows < to change the stdin descriptor when starting a nuttx task or to set the save/restore strucutures on the nsh handling.

I am not sure if this is the correct way to achieve this, so I open this PR to get feedback from the community. Please let me know if there is a better way or if I am misunderstanding or missing something.

I also plan to support << style heredoc and pipes |. But those are not implemented yet.

If I get positive feedback, I could implement those in a future PR.

Partially fixes #2454

Impact

Now we can stdin and stdout redirection:

nsh> echo hello > oi
nsh> cat < oi
hello
nsh> cat < oi > ola
nsh> cat ola
hello
nsh> echo even works the other way around > oi2
nsh> cat > new < oi2
nsh> cat new
even works the other way around

Testing

Manual testing. Still need to test how it works in telnet, usb console or altconsole.

nshlib/nsh_parse.c

This adds support for `<` to redirect input on nsh commands.

This allows programs such as `cat` to read from the console (that could be redirected).

Now, if we run cat without arguments, it will just read from stdin. It can be used with redirect like `cat < infile > outfile`.

acassis · 2024-08-07T18:26:29Z

@xiaoxiang781216 any idea why is ARM failing?

xiaoxiang781216 · 2024-08-08T01:54:14Z

@xiaoxiang781216 any idea why is ARM failing?

The cod size is bigger than flash capacity(208B) of lm3s6965-ek/qemu-protected:

====================================================================================
Configuration/Tool: lm3s6965-ek/qemu-protected,CONFIG_ARM_TOOLCHAIN_GNU_EABI
2024-08-07 16:06:03
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
  Disabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
  Enabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
  Building NuttX...
arm-none-eabi-ld: nuttx_user.elf section `.text' will not fit in region `uflash'
arm-none-eabi-ld: region `uflash' overflowed by 208 bytes
make[1]: *** [Makefile:59: nuttx_user.elf] Error 1
make[1]: Target 'all' not remade because of errors.
make: *** [tools/Unix.mk:535: nuttx] Error 2
make: Target 'all' not remade because of errors.
/github/workspace/sources/nuttx/tools/testbuild.sh: line 378: /github/workspace/sources/nuttx/../nuttx/nuttx.manifest: No such file or directory
  Normalize lm3s6965-ek/qemu-protected

casaroli · 2024-08-08T17:13:43Z

Added the support for fileapps, which was missing.

acassis · 2024-08-08T22:19:14Z

I will merge and fix the lm3s6965-ek/qemu-protected into nuttx mainline

Revert "nsh_fileapp: handle input redirection" This reverts commit e46347e. Revert "feat(nsh_cat): allow cat to read from stdin" This reverts commit 8fba726. Revert "feat(nsh): add console read" This reverts commit 4104019. Revert "feat(nsh): input (stdin) redirection" This reverts commit 96100b3.

tmedicci · 2024-08-09T15:15:35Z

Please, @acassis @xiaoxiang781216 and @casaroli, check #2474 ASAP.

Due to the urgency of this issue, I recommend reverting the commits from this PR to let @casaroli properly evaluate and fix the problem.

acassis · 2024-08-09T15:30:40Z

@tmedicci the issue is with the lm3s6965-ek qemu-protected, it enabled more features than necessary, invested of reverting I suggest just doing it:

-CONFIG_SYSTEM_NTPC=y

tmedicci · 2024-08-09T16:46:53Z

CONFIG_SYSTEM_NTPC

This config is not even enabled in the defconfigs I found the issue, @acassis

tmedicci · 2024-08-09T16:53:54Z

CONFIG_SYSTEM_NTPC

This config is not even enabled in the defconfigs I found the issue, @acassis

BTW, the issue happens in all Espressif's defconfig and in the simulator (sim:nsh).

The PR apache#2469 broke handling of environment variables because an error in the if/else if control flow. This fixes it.

The PR #2469 broke handling of environment variables because an error in the if/else if control flow. This fixes it.

JianyuWang0623 · 2024-10-11T14:24:09Z

@casaroli Thank you. it`s very helpful.
Do you have a specific plan to implement pipeline?

Board/Config/Command sim:nsh cat < /etc/init.d/rc.sysinit Problem The NSH hangs after exec the test command. `nsh_redirect()` needs to save three fd but array length is only two, stack buffer overflowed! Related: apache#2469 Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>

Board/Config/Command sim:nsh cat < /etc/init.d/rc.sysinit Problem The NSH hangs after exec the test command. `nsh_redirect()` needs to save three fd but array length is only two, stack buffer overflowed! Related: #2469 Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>

casaroli force-pushed the nsh-redirect-stdin branch 2 times, most recently from 279bea4 to 4fb8665 Compare August 6, 2024 22:14

xiaoxiang781216 reviewed Aug 7, 2024

View reviewed changes

casaroli force-pushed the nsh-redirect-stdin branch from 4fb8665 to 551dcba Compare August 7, 2024 06:55

casaroli requested a review from xiaoxiang781216 August 7, 2024 07:40

xiaoxiang781216 marked this pull request as ready for review August 7, 2024 07:49

casaroli force-pushed the nsh-redirect-stdin branch from 551dcba to d4bfdbb Compare August 7, 2024 13:36

casaroli added 3 commits August 7, 2024 15:37

feat(nsh): input (stdin) redirection

f38494d

This adds support for `<` to redirect input on nsh commands.

feat(nsh): add console read

b279c8b

This allows programs such as `cat` to read from the console (that could be redirected).

feat(nsh_cat): allow cat to read from stdin

52c4b76

Now, if we run cat without arguments, it will just read from stdin. It can be used with redirect like `cat < infile > outfile`.

casaroli force-pushed the nsh-redirect-stdin branch from d4bfdbb to 52c4b76 Compare August 7, 2024 13:37

xiaoxiang781216 approved these changes Aug 7, 2024

View reviewed changes

jerpelea approved these changes Aug 7, 2024

View reviewed changes

acassis approved these changes Aug 7, 2024

View reviewed changes

nsh_fileapp: handle input redirection

44ac8b6

casaroli force-pushed the nsh-redirect-stdin branch from 68529af to 44ac8b6 Compare August 8, 2024 17:15

xiaoxiang781216 approved these changes Aug 8, 2024

View reviewed changes

acassis merged commit e46347e into apache:master Aug 8, 2024

tmedicci mentioned this pull request Aug 9, 2024

Revert commits from PR #2469 #2474

Closed

casaroli added a commit to casaroli/nuttx-apps that referenced this pull request Aug 10, 2024

fix(nsh_parse): handle env variables correctly

e54cebd

The PR apache#2469 broke handling of environment variables because an error in the if/else if control flow. This fixes it.

casaroli mentioned this pull request Aug 10, 2024

fix(nsh_parse): handle env variables correctly #2475

Merged

acassis pushed a commit that referenced this pull request Aug 10, 2024

fix(nsh_parse): handle env variables correctly

0797ee2

The PR #2469 broke handling of environment variables because an error in the if/else if control flow. This fixes it.

JianyuWang0623 mentioned this pull request Oct 12, 2024

nshlib: Fix stack-buffer-overflow of nsh_redirect() #2699

Merged

JianyuWang0623 mentioned this pull request Oct 15, 2024

Add pipeline support for nsh commandline #2737

Merged

nsh: input redirects #2469

nsh: input redirects #2469

Uh oh!

Conversation

casaroli commented Aug 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Impact

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acassis commented Aug 7, 2024

Uh oh!

xiaoxiang781216 commented Aug 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

casaroli commented Aug 8, 2024

Uh oh!

acassis commented Aug 8, 2024

Uh oh!

tmedicci commented Aug 9, 2024

Uh oh!

acassis commented Aug 9, 2024

Uh oh!

tmedicci commented Aug 9, 2024

Uh oh!

tmedicci commented Aug 9, 2024

Uh oh!

JianyuWang0623 commented Oct 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

casaroli commented Aug 6, 2024 •

edited

Loading

xiaoxiang781216 commented Aug 8, 2024 •

edited

Loading