Ilari Liusvaara <ilari.liusvaara@xxxxxxxxxxx> writes: > Previously run-command was unable to report errors happening in exec > call. Change it to pass errno from failed exec to errno value at > return. > > The errno value passing can be done by opening close-on-exec pipe and > piping the error code through in case of failure. In case of success, > close-on-exec closes the pipe on successful exec and parent process > gets end of file on read. Thanks; I think overall this is a good idea, even though I have no clue if WIN32 side wants a similar support. - At first reading, the "while (close(fd) < 0 && errno != EBADF);" pattern was a bit of eyesore. It might be worth factoring that out to a small static helper function that a smart compiler would automatically inline (or mark it as a static inline). - Is it guaranteed that a failed pipe(2) never touches its int fildes[2] parameter, or the values are undefined when it fails? The approach would save one extra variable, compared to an alternative approach to keep an explicit variable to record a pipe failure, but It feels iffy that the code relies on them being left as their initial -1 values. - Should we worry about partial write as well (you seem to warn when you get a partial read)? Is it worth using xread() and friends found in wrapper.c instead of goto read/write_again? - Shouldn't any of these warning() be die() instead? - The two extra file descriptors this patch uses are allocated after all the existing pipe flipping is done, and all the dup's done in the child process are to use dup2() to set up the known file descriptors at low numbers, so I don't think we have to worry about this patch changing the behaviour of the process pair by changing the assignment of file descriptors (we had a bug or two in the past that made subprocess misbehave under some funky condition, e.g. run with fd#0 closed). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html