xiaoxiang781216 commented on a change in pull request #521:
URL:
https://github.com/apache/incubator-nuttx-apps/pull/521#discussion_r542427013
##########
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:
> > But, I am curious that line 87-89 already duplicate fd to 0-2, why
we still need call fdopen at line 98-100 again?
>
> The file descriptors are duplicated, but the the streams are still not
opened due to the failure of fs_fdopen() when the IDLE thread was created.
Duplicating the file descriptor does not create the stream.
>
Ok, I understand the situation now.
> This should not be done in the the OS (not the application) OR it could be
done in _all_ applications and the non-standard requirement should be well
documented.
Maybe the right thing we should do:
1. Always enable DEV_ZERO or DEV_NULL
2. Or skip check 0-2 valid in fs_fdopen
The second approach is reasonable since VFS will always ensure 0-2 is valid
when the usespace issue the file system operation. And then we can remove
fs_fdopen too.
> The latter is basically the way things worked before your change that
broke this.
But with the patch: https://github.com/apache/incubator-nuttx/pull/2263
I think the new behaviour is same as before now. If CONFIG_DEV_CONSOLE isn't
enabled, fdopen is always required regardless whether my patch apply.
----------------------------------------------------------------
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
