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 2021/10/31 13:45:26 UTC
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 opened a new pull request #873: nshlib/nsh_session: Handle the command arguments in any order
xiaoxiang781216 opened a new pull request #873:
URL: https://github.com/apache/incubator-nuttx-apps/pull/873
## Summary
Make the command sh command line handling more robust
## Impact
## Testing
run sh with script
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] gustavonihei merged pull request #873: nshlib/nsh_session: Handle the command arguments in any order
Posted by GitBox <gi...@apache.org>.
gustavonihei merged pull request #873:
URL: https://github.com/apache/incubator-nuttx-apps/pull/873
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] gustavonihei commented on a change in pull request #873: nshlib/nsh_session: Handle the command arguments in any order
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #873:
URL: https://github.com/apache/incubator-nuttx-apps/pull/873#discussion_r744748266
##########
File path: nshlib/nsh_session.c
##########
@@ -113,91 +114,112 @@ int nsh_session(FAR struct console_stdio_s *pstate,
#endif
}
- if (argc < 2)
- {
- /* Then enter the command line parsing loop */
+ /* Process the command line option */
- for (; ; )
+ for (i = 1; i < argc; i++)
+ {
+ if (strcmp(argv[i], "-h") == 0)
{
- /* For the case of debugging the USB console...
- * dump collected USB trace data
- */
-
-#ifdef CONFIG_NSH_USBDEV_TRACE
- nsh_usbtrace();
-#endif
-
- /* Get the next line of input. readline() returns EOF
- * on end-of-file or any read failure.
- */
-
-#ifdef CONFIG_NSH_CLE
- /* cle() normally returns the number of characters read, but will
- * return a negated errno value on end of file or if an error
- * occurs. Either will cause the session to terminate.
- */
+ nsh_output(vtbl, "Usage: %s [<script-path>|-c <command>]\n",
+ argv[0]);
+ return EXIT_SUCCESS;
+ }
+ else if (strcmp(argv[i], "-c") == 0)
+ {
+ /* Process the inline command */
- ret = cle(pstate->cn_line, g_nshprompt, CONFIG_NSH_LINELEN,
- INSTREAM(pstate), OUTSTREAM(pstate));
- if (ret < 0)
+ if (i + 1 < argc)
{
- fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
- "cle", NSH_ERRNO_OF(-ret));
- continue;
+ return nsh_parse(vtbl, argv[i + 1]);
}
-#else
- /* Display the prompt string */
-
- fputs(g_nshprompt, pstate->cn_outstream);
- fflush(pstate->cn_outstream);
-
- /* readline() normally returns the number of characters read, but
- * will return EOF on end of file or if an error occurs. EOF
- * will cause the session to terminate.
- */
-
- ret = readline(pstate->cn_line, CONFIG_NSH_LINELEN,
- INSTREAM(pstate), OUTSTREAM(pstate));
- if (ret == EOF)
+ else
{
- /* NOTE: readline() does not set the errno variable, but
- * perhaps we will be lucky and it will still be valid.
- */
-
- fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
- "readline", NSH_ERRNO);
- ret = EXIT_SUCCESS;
- break;
+ nsh_error(vtbl, g_fmtargrequired, argv[0]);
+ return EXIT_FAILURE;
}
-#endif
+ }
+ else if (argv[i][0] != '-')
+ {
+ break;
+ }
- /* Parse process the command */
+ /* Unknown option */
- nsh_parse(vtbl, pstate->cn_line);
- fflush(pstate->cn_outstream);
- }
- }
- else if (strcmp(argv[1], "-h") == 0)
- {
- ret = nsh_output(vtbl, "Usage: %s [<script-path>|-c <command>]\n",
- argv[0]);
+ nsh_error(vtbl, g_fmtsyntax, argv[0]);
+ return EXIT_FAILURE;
}
- else if (strcmp(argv[1], "-c") != 0)
+
+ if (i < argc)
{
#if defined(CONFIG_FILE_STREAM) && !defined(CONFIG_NSH_DISABLESCRIPT)
/* Execute the shell script */
- ret = nsh_script(vtbl, argv[0], argv[1]);
+ return nsh_script(vtbl, argv[0], argv[i]);
+#else
+ return EXIT_FAILURE;
#endif
}
- else if (argc >= 3)
+
+ /* Then enter the command line parsing loop */
+
+ for (; ; )
{
- /* Parse process the command */
+ /* For the case of debugging the USB console...
+ * dump collected USB trace data
+ */
- ret = nsh_parse(vtbl, argv[2]);
-#ifdef CONFIG_FILE_STREAM
+#ifdef CONFIG_NSH_USBDEV_TRACE
+ nsh_usbtrace();
+#endif
+
+ /* Get the next line of input. readline() returns EOF
+ * on end-of-file or any read failure.
+ */
+
+#ifdef CONFIG_NSH_CLE
+ /* cle() normally returns the number of characters read, but will
+ * return a negated errno value on end of file or if an error
+ * occurs. Either will cause the session to terminate.
+ */
+
+ ret = cle(pstate->cn_line, g_nshprompt, CONFIG_NSH_LINELEN,
+ INSTREAM(pstate), OUTSTREAM(pstate));
+ if (ret < 0)
+ {
+ fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
+ "cle", NSH_ERRNO_OF(-ret));
+ continue;
+ }
+#else
+ /* Display the prompt string */
+
+ fputs(g_nshprompt, pstate->cn_outstream);
fflush(pstate->cn_outstream);
Review comment:
The check for the definition of `CONFIG_FILE_STREAM` has been removed. It seems that the verification is somewhat inconsistent in this function, maybe because it is expected to be always enabled.
If so, the check for `CONFIG_FILE_STREAM` on line 154 could be removed as well.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] gustavonihei commented on a change in pull request #873: nshlib/nsh_session: Handle the command arguments in any order
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #873:
URL: https://github.com/apache/incubator-nuttx-apps/pull/873#discussion_r744885229
##########
File path: nshlib/nsh_session.c
##########
@@ -113,91 +114,112 @@ int nsh_session(FAR struct console_stdio_s *pstate,
#endif
}
- if (argc < 2)
- {
- /* Then enter the command line parsing loop */
+ /* Process the command line option */
- for (; ; )
+ for (i = 1; i < argc; i++)
+ {
+ if (strcmp(argv[i], "-h") == 0)
{
- /* For the case of debugging the USB console...
- * dump collected USB trace data
- */
-
-#ifdef CONFIG_NSH_USBDEV_TRACE
- nsh_usbtrace();
-#endif
-
- /* Get the next line of input. readline() returns EOF
- * on end-of-file or any read failure.
- */
-
-#ifdef CONFIG_NSH_CLE
- /* cle() normally returns the number of characters read, but will
- * return a negated errno value on end of file or if an error
- * occurs. Either will cause the session to terminate.
- */
+ nsh_output(vtbl, "Usage: %s [<script-path>|-c <command>]\n",
+ argv[0]);
+ return EXIT_SUCCESS;
+ }
+ else if (strcmp(argv[i], "-c") == 0)
+ {
+ /* Process the inline command */
- ret = cle(pstate->cn_line, g_nshprompt, CONFIG_NSH_LINELEN,
- INSTREAM(pstate), OUTSTREAM(pstate));
- if (ret < 0)
+ if (i + 1 < argc)
{
- fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
- "cle", NSH_ERRNO_OF(-ret));
- continue;
+ return nsh_parse(vtbl, argv[i + 1]);
}
-#else
- /* Display the prompt string */
-
- fputs(g_nshprompt, pstate->cn_outstream);
- fflush(pstate->cn_outstream);
-
- /* readline() normally returns the number of characters read, but
- * will return EOF on end of file or if an error occurs. EOF
- * will cause the session to terminate.
- */
-
- ret = readline(pstate->cn_line, CONFIG_NSH_LINELEN,
- INSTREAM(pstate), OUTSTREAM(pstate));
- if (ret == EOF)
+ else
{
- /* NOTE: readline() does not set the errno variable, but
- * perhaps we will be lucky and it will still be valid.
- */
-
- fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
- "readline", NSH_ERRNO);
- ret = EXIT_SUCCESS;
- break;
+ nsh_error(vtbl, g_fmtargrequired, argv[0]);
+ return EXIT_FAILURE;
}
-#endif
+ }
+ else if (argv[i][0] != '-')
+ {
+ break;
+ }
- /* Parse process the command */
+ /* Unknown option */
- nsh_parse(vtbl, pstate->cn_line);
- fflush(pstate->cn_outstream);
- }
- }
- else if (strcmp(argv[1], "-h") == 0)
- {
- ret = nsh_output(vtbl, "Usage: %s [<script-path>|-c <command>]\n",
- argv[0]);
+ nsh_error(vtbl, g_fmtsyntax, argv[0]);
+ return EXIT_FAILURE;
}
- else if (strcmp(argv[1], "-c") != 0)
+
+ if (i < argc)
{
#if defined(CONFIG_FILE_STREAM) && !defined(CONFIG_NSH_DISABLESCRIPT)
/* Execute the shell script */
- ret = nsh_script(vtbl, argv[0], argv[1]);
+ return nsh_script(vtbl, argv[0], argv[i]);
+#else
+ return EXIT_FAILURE;
#endif
}
- else if (argc >= 3)
+
+ /* Then enter the command line parsing loop */
+
+ for (; ; )
{
- /* Parse process the command */
+ /* For the case of debugging the USB console...
+ * dump collected USB trace data
+ */
- ret = nsh_parse(vtbl, argv[2]);
-#ifdef CONFIG_FILE_STREAM
+#ifdef CONFIG_NSH_USBDEV_TRACE
+ nsh_usbtrace();
+#endif
+
+ /* Get the next line of input. readline() returns EOF
+ * on end-of-file or any read failure.
+ */
+
+#ifdef CONFIG_NSH_CLE
+ /* cle() normally returns the number of characters read, but will
+ * return a negated errno value on end of file or if an error
+ * occurs. Either will cause the session to terminate.
+ */
+
+ ret = cle(pstate->cn_line, g_nshprompt, CONFIG_NSH_LINELEN,
+ INSTREAM(pstate), OUTSTREAM(pstate));
+ if (ret < 0)
+ {
+ fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
+ "cle", NSH_ERRNO_OF(-ret));
+ continue;
+ }
+#else
+ /* Display the prompt string */
+
+ fputs(g_nshprompt, pstate->cn_outstream);
fflush(pstate->cn_outstream);
Review comment:
I've just found that `nsh_session.c` is only built when `CONFIG_FILE_STREAM=y`:
https://github.com/apache/incubator-nuttx-apps/blob/master/nshlib/Makefile#L29-L39
Weird, though, is that `nsh_stdsession.c` is built otherwise. But portions of the source file still check for `CONFIG_FILE_STREAM`. I wonder if that is being used anywhere.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #873: nshlib/nsh_session: Handle the command arguments in any order
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #873:
URL: https://github.com/apache/incubator-nuttx-apps/pull/873#discussion_r744856834
##########
File path: nshlib/nsh_session.c
##########
@@ -113,91 +114,112 @@ int nsh_session(FAR struct console_stdio_s *pstate,
#endif
}
- if (argc < 2)
- {
- /* Then enter the command line parsing loop */
+ /* Process the command line option */
- for (; ; )
+ for (i = 1; i < argc; i++)
+ {
+ if (strcmp(argv[i], "-h") == 0)
{
- /* For the case of debugging the USB console...
- * dump collected USB trace data
- */
-
-#ifdef CONFIG_NSH_USBDEV_TRACE
- nsh_usbtrace();
-#endif
-
- /* Get the next line of input. readline() returns EOF
- * on end-of-file or any read failure.
- */
-
-#ifdef CONFIG_NSH_CLE
- /* cle() normally returns the number of characters read, but will
- * return a negated errno value on end of file or if an error
- * occurs. Either will cause the session to terminate.
- */
+ nsh_output(vtbl, "Usage: %s [<script-path>|-c <command>]\n",
+ argv[0]);
+ return EXIT_SUCCESS;
+ }
+ else if (strcmp(argv[i], "-c") == 0)
+ {
+ /* Process the inline command */
- ret = cle(pstate->cn_line, g_nshprompt, CONFIG_NSH_LINELEN,
- INSTREAM(pstate), OUTSTREAM(pstate));
- if (ret < 0)
+ if (i + 1 < argc)
{
- fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
- "cle", NSH_ERRNO_OF(-ret));
- continue;
+ return nsh_parse(vtbl, argv[i + 1]);
}
-#else
- /* Display the prompt string */
-
- fputs(g_nshprompt, pstate->cn_outstream);
- fflush(pstate->cn_outstream);
-
- /* readline() normally returns the number of characters read, but
- * will return EOF on end of file or if an error occurs. EOF
- * will cause the session to terminate.
- */
-
- ret = readline(pstate->cn_line, CONFIG_NSH_LINELEN,
- INSTREAM(pstate), OUTSTREAM(pstate));
- if (ret == EOF)
+ else
{
- /* NOTE: readline() does not set the errno variable, but
- * perhaps we will be lucky and it will still be valid.
- */
-
- fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
- "readline", NSH_ERRNO);
- ret = EXIT_SUCCESS;
- break;
+ nsh_error(vtbl, g_fmtargrequired, argv[0]);
+ return EXIT_FAILURE;
}
-#endif
+ }
+ else if (argv[i][0] != '-')
+ {
+ break;
+ }
- /* Parse process the command */
+ /* Unknown option */
- nsh_parse(vtbl, pstate->cn_line);
- fflush(pstate->cn_outstream);
- }
- }
- else if (strcmp(argv[1], "-h") == 0)
- {
- ret = nsh_output(vtbl, "Usage: %s [<script-path>|-c <command>]\n",
- argv[0]);
+ nsh_error(vtbl, g_fmtsyntax, argv[0]);
+ return EXIT_FAILURE;
}
- else if (strcmp(argv[1], "-c") != 0)
+
+ if (i < argc)
{
#if defined(CONFIG_FILE_STREAM) && !defined(CONFIG_NSH_DISABLESCRIPT)
/* Execute the shell script */
- ret = nsh_script(vtbl, argv[0], argv[1]);
+ return nsh_script(vtbl, argv[0], argv[i]);
+#else
+ return EXIT_FAILURE;
#endif
}
- else if (argc >= 3)
+
+ /* Then enter the command line parsing loop */
+
+ for (; ; )
{
- /* Parse process the command */
+ /* For the case of debugging the USB console...
+ * dump collected USB trace data
+ */
- ret = nsh_parse(vtbl, argv[2]);
-#ifdef CONFIG_FILE_STREAM
+#ifdef CONFIG_NSH_USBDEV_TRACE
+ nsh_usbtrace();
+#endif
+
+ /* Get the next line of input. readline() returns EOF
+ * on end-of-file or any read failure.
+ */
+
+#ifdef CONFIG_NSH_CLE
+ /* cle() normally returns the number of characters read, but will
+ * return a negated errno value on end of file or if an error
+ * occurs. Either will cause the session to terminate.
+ */
+
+ ret = cle(pstate->cn_line, g_nshprompt, CONFIG_NSH_LINELEN,
+ INSTREAM(pstate), OUTSTREAM(pstate));
+ if (ret < 0)
+ {
+ fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
+ "cle", NSH_ERRNO_OF(-ret));
+ continue;
+ }
+#else
+ /* Display the prompt string */
+
+ fputs(g_nshprompt, pstate->cn_outstream);
fflush(pstate->cn_outstream);
Review comment:
> The check for the definition of `CONFIG_FILE_STREAM` has been removed. It seems that the verification is somewhat inconsistent in this function, maybe because it is expected to be always enabled.
Yes, your finding is right. You can see that there are many place call puts/fprintf without the guard of CONFIG_FILE_STREAM. That's why I remove `CONFIG_FILE_STREAM`.
> If so, the check for `CONFIG_FILE_STREAM` on line 154 could be removed as well.
Done.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #873: nshlib/nsh_session: Handle the command arguments in any order
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #873:
URL: https://github.com/apache/incubator-nuttx-apps/pull/873#discussion_r744857236
##########
File path: nshlib/nsh_session.c
##########
@@ -113,91 +114,112 @@ int nsh_session(FAR struct console_stdio_s *pstate,
#endif
}
- if (argc < 2)
- {
- /* Then enter the command line parsing loop */
+ /* Process the command line option */
- for (; ; )
+ for (i = 1; i < argc; i++)
+ {
+ if (strcmp(argv[i], "-h") == 0)
{
- /* For the case of debugging the USB console...
- * dump collected USB trace data
- */
-
-#ifdef CONFIG_NSH_USBDEV_TRACE
- nsh_usbtrace();
-#endif
-
- /* Get the next line of input. readline() returns EOF
- * on end-of-file or any read failure.
- */
-
-#ifdef CONFIG_NSH_CLE
- /* cle() normally returns the number of characters read, but will
- * return a negated errno value on end of file or if an error
- * occurs. Either will cause the session to terminate.
- */
+ nsh_output(vtbl, "Usage: %s [<script-path>|-c <command>]\n",
+ argv[0]);
+ return EXIT_SUCCESS;
+ }
+ else if (strcmp(argv[i], "-c") == 0)
+ {
+ /* Process the inline command */
- ret = cle(pstate->cn_line, g_nshprompt, CONFIG_NSH_LINELEN,
- INSTREAM(pstate), OUTSTREAM(pstate));
- if (ret < 0)
+ if (i + 1 < argc)
{
- fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
- "cle", NSH_ERRNO_OF(-ret));
- continue;
+ return nsh_parse(vtbl, argv[i + 1]);
}
-#else
- /* Display the prompt string */
-
- fputs(g_nshprompt, pstate->cn_outstream);
- fflush(pstate->cn_outstream);
-
- /* readline() normally returns the number of characters read, but
- * will return EOF on end of file or if an error occurs. EOF
- * will cause the session to terminate.
- */
-
- ret = readline(pstate->cn_line, CONFIG_NSH_LINELEN,
- INSTREAM(pstate), OUTSTREAM(pstate));
- if (ret == EOF)
+ else
{
- /* NOTE: readline() does not set the errno variable, but
- * perhaps we will be lucky and it will still be valid.
- */
-
- fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
- "readline", NSH_ERRNO);
- ret = EXIT_SUCCESS;
- break;
+ nsh_error(vtbl, g_fmtargrequired, argv[0]);
+ return EXIT_FAILURE;
}
-#endif
+ }
+ else if (argv[i][0] != '-')
+ {
+ break;
+ }
- /* Parse process the command */
+ /* Unknown option */
- nsh_parse(vtbl, pstate->cn_line);
- fflush(pstate->cn_outstream);
- }
- }
- else if (strcmp(argv[1], "-h") == 0)
- {
- ret = nsh_output(vtbl, "Usage: %s [<script-path>|-c <command>]\n",
- argv[0]);
+ nsh_error(vtbl, g_fmtsyntax, argv[0]);
+ return EXIT_FAILURE;
}
- else if (strcmp(argv[1], "-c") != 0)
+
+ if (i < argc)
{
#if defined(CONFIG_FILE_STREAM) && !defined(CONFIG_NSH_DISABLESCRIPT)
/* Execute the shell script */
- ret = nsh_script(vtbl, argv[0], argv[1]);
+ return nsh_script(vtbl, argv[0], argv[i]);
+#else
+ return EXIT_FAILURE;
#endif
}
- else if (argc >= 3)
+
+ /* Then enter the command line parsing loop */
+
+ for (; ; )
{
- /* Parse process the command */
+ /* For the case of debugging the USB console...
+ * dump collected USB trace data
+ */
- ret = nsh_parse(vtbl, argv[2]);
-#ifdef CONFIG_FILE_STREAM
+#ifdef CONFIG_NSH_USBDEV_TRACE
+ nsh_usbtrace();
+#endif
+
+ /* Get the next line of input. readline() returns EOF
+ * on end-of-file or any read failure.
+ */
+
+#ifdef CONFIG_NSH_CLE
+ /* cle() normally returns the number of characters read, but will
+ * return a negated errno value on end of file or if an error
+ * occurs. Either will cause the session to terminate.
+ */
+
+ ret = cle(pstate->cn_line, g_nshprompt, CONFIG_NSH_LINELEN,
+ INSTREAM(pstate), OUTSTREAM(pstate));
+ if (ret < 0)
+ {
+ fprintf(pstate->cn_errstream, g_fmtcmdfailed, "nsh_session",
+ "cle", NSH_ERRNO_OF(-ret));
+ continue;
+ }
+#else
+ /* Display the prompt string */
+
+ fputs(g_nshprompt, pstate->cn_outstream);
fflush(pstate->cn_outstream);
Review comment:
> The check for the definition of `CONFIG_FILE_STREAM` has been removed. It seems that the verification is somewhat inconsistent in this function, maybe because it is expected to be always enabled.
Yes, your finding is right. You can see that there are many place call puts/fprintf without the guard of CONFIG_FILE_STREAM. That's why I remove `CONFIG_FILE_STREAM`.
> If so, the check for `CONFIG_FILE_STREAM` on line 154 could be removed as well.
Done.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org