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
Subsequently in fdopen() calls in group_setupstreams() will fail in
fs/vfs/fd_open() because the file descriptor check will fail, fs_chedfd().
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. Otherwise, EVERY application that works
with no initial, default console must use this same non-standard,
non-documented trick.
----------------------------------------------------------------
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:
us...@infra.apache.org
