[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #521: nshlib: Fix nsh_usbconsole.c

[GitBox <gi...@apache.org>]

patacongo commented on a change in pull request #521:
URL: https://github.com/apache/incubator-nuttx-apps/pull/521#discussion_r542348706



##########
File path: nshlib/nsh_usbconsole.c
##########
@@ -88,15 +88,16 @@ static void nsh_configstdio(int fd, FAR struct console_stdio_s *pstate)
   dup2(fd, 1);
   dup2(fd, 2);
 
-  /* Setup the stdout */
-
-  pstate->cn_outfd     = 1;
-  pstate->cn_outstream = fdopen(1, "a");
-
-  /* Setup the stderr */
+  /* fdopen to get the stdin, stdout and stderr streams.
+   *
+   * fd = 0 is stdin  (read-only)
+   * fd = 1 is stdout (write-only, append)
+   * fd = 2 is stderr (write-only, append)
+   */
 
-  pstate->cn_errfd     = 2;
-  pstate->cn_errstream = fdopen(2, "a");
+  fdopen(0, "r");
+  fdopen(1, "a");
+  fdopen(2, "a");

Review comment:
       This is generally not needed.  The fact that fdopen() corrects the problem is an artifact of the fact that CONFIG_DEV_CONSOLE is not set in the configuration.
   
   See sched/group/goupr_setupidlefiles.c:
   
        98 #ifdef CONFIG_DEV_CONSOLE
        99   fd = nx_open("/dev/console", O_RDWR);
       100   if (fd == 0)
       101     {
       102       /* Successfully opened /dev/console as stdin (fd == 0) */
       103
       104       fs_dupfd2(0, 1);
       105       fs_dupfd2(0, 2);
       106     }
       125 #endif
   
   I believe that it is the logic in sched/group/goupr_setupidlefiles.c.  Since stdin, stdout, and stderr must always be available, it is wrong to permit CONFIG_DEV_CONSOLE to not be set.
   
   A better solution would be to use /dev/null for tstdin, stdout, and stderr if CONFIG_DEV_CONSOLE  is not set.
   




----------------------------------------------------------------
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