You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/04/18 11:19:16 UTC

[GitHub] [incubator-nuttx-apps] xiaoxiang781216 edited a comment on issue #185: nsh improvement

xiaoxiang781216 edited a comment on issue #185: nsh improvement
URL: https://github.com/apache/incubator-nuttx-apps/pull/185#issuecomment-615844029
 
 
   > There is a logic flaw in hsow system() is called. The system() call will block until the cmd is executed. Any & or redirection on the command will not be handled. Too much breakage! That is not acceptable and must be fixed.
   > 
   
   But the old implementation call nsh_script directly, which also block until the script finish execution. If the old behaviour support &, then the new should support too, because the only difference is that the new one has a longer call chain:
   new: cmd_sh->system->posix_task->nsh_system->nsh_parse->nsh_source->nsh_script
   old: cmd_sh->nsh_script
   The real action happen inside nsh_script, so the key is that nsh_script need to enhance to support &.
   
   > Hmmm... Or does the system command run on a pthread via cmd_sh(). In that case, then the & background option would work (although it ties up both a pthread and a task).
   > 
   
   Should we put the logic into nsh_script? since we have support:
   sh /path/to/script &
   source /path/to/script &
   Both will call nsh_script finally.
   
   > But the redirection as it is implemented for cmd_sh() will not work for system(cmd). Within NSH, redirection is handled via nsh_output(), stdin/stdout/stderr are not changed.
   > 
   
   Yes, if the redirection just change the internal parser state not stdin/stdtout/stderr, the new nsh task can't inherit the redirection setting.
   
   > So even though you redirect the output at cmd_sh, will not be redirected by system(). This is not good. This is a good idea but not ready to come into the master.
   
   How about we modify stdin/stdout/stderr directly?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services