Make stdin a psuedoterminal when isatty is set #1265
Conversation
| if (stdin_fd == -1) { | ||
| fprintf(stderr, "open: %s: %s\n", argv[1], strerror(errno)); | ||
| return 127; | ||
| // Because we want to be able to read in both file descirptors and |
| if (stdin_fd == -1) { | ||
| fprintf(stderr, "open: %s: %s\n", argv[1], strerror(errno)); | ||
| return 127; |
There was a problem hiding this comment.
Please move this if after the if/else in case atoi fails for some reason
There was a problem hiding this comment.
atoi does not tell you if it fails so we can't catch the error :(
There was a problem hiding this comment.
I just meant something like
if (stdin_file[0] == '#') {
stdin_fd = atoi(stdin_file + 1);
} else {
stdin_fd = open(stdin_file, O_RDONLY);
}
if (stdin_fd == -1) {
fprintf(stderr, "open: %s: %s\n", argv[1], strerror(errno));
return 127;
}
There was a problem hiding this comment.
But that's incorrect for the atoi case. The dup/dup2 below will fail if something is wrong with atoi....maybe the reality is that we should not being using atoi because its a terrible interface lol.
| @@ -829,9 +829,17 @@ static void launch(JobTable *jobtable) { | |||
| jobtable->imp->pipes[stdout_stream[0]] = entry; | |||
| jobtable->imp->pipes[stderr_stream[0]] = entry; | |||
| clock_gettime(CLOCK_REALTIME, &entry->job->start); | |||
| std::string stdin_file_str = "/dev/null"; | |||
| int stdin_stream[2]; | |||
There was a problem hiding this comment.
I'm not going to block, but I really think this should be explicit via a tuple data in wakelang.
There was a problem hiding this comment.
What do you mean exactly?
There was a problem hiding this comment.
Mentioned in chat but something like
data StdInOption =
DevNull
Interactive
File (filename)
def job =
makeExecPlan ......
| setPlanStdinOption DevNull
| runJob
Sometimes programs check isatty(STDIN_FILENO) in addition to stderr and stdout to determine if they're using a terminal or not. We should support that case as well as we can. We'd need to perform some surgery on shim-wake to support the case where stdin is a file input but it ALSO has input. So for now we treat setting stdin to a file to be just that and not a pseudo-terminal.