bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly

2021-12-28 Thread Bug reports for GUILE, GNU's Ubiquitous Extension Language
Hello Timothy,

Timothy Sample  writes:

> One thing that ‘renumber_file_descriptor’ does that we seem to be losing
> here is error checking.  To my eye, the old code will try and warn the
> user if they run out of file descriptors, but the new code will not.

I may have been too hasty on that front, you're right that we should
check for errors when dup'ing.  Note though that we should not run out
of fds, as we close all of them except for in/out/err right before using
it, but still, here are the interesting error codes:
* EINTR, although OpenBSD and Linux will never set that error from my
light research;
* EBUSY, not POSIX though, only Linux, so should we?
* EMFILE, if we run out of fds;
* EBADF if either of the fds are invalid.

As for error reporting, should we keep the parent's stderr fd open while
we do all of this, just to be able to report an error in case it
happens?  The old code used to try reporting the errors to the new `err`
fd instead, but it might be clearer to use stderr.

WDYT?

-- 
Josselin Poiret





bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly

2021-12-28 Thread Timothy Sample
Hey Josselin,

Thanks for finding this bug!  I have one concern about your patches:

Josselin Poiret writes:

> The second patch removes renumber_file_descriptor, as it is no longer
> used.

One thing that ‘renumber_file_descriptor’ does that we seem to be losing
here is error checking.  To my eye, the old code will try and warn the
user if they run out of file descriptors, but the new code will not.

The other thing that I like is how ‘renumber_file_descriptor’ checks the
return value of ‘dup’ in addition to checking ‘errno’.  (I realize that
the old code skips that check for ‘dup2’ – I’m kinda just stating a
preference here.)  A quick check of POSIX turns up the following: “the
value of ‘errno’ should only be examined when it is indicated to be
valid by a function’s return value” [1].


-- Tim

[1] https://pubs.opengroup.org/onlinepubs/9699919799/