This is the mail archive of the cygwin-patches mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
According to Corinna Vinschen on 1/14/2010 3:25 AM: >>> + if (flags & O_CLOEXEC) >>> + newfh->set_close_on_exec (true); >> Is that a typo? >> >>> + else >>> + newfh->close_on_exec (false); >> If not, why not just newfh->close_on_exec (!!(flags & O_CLOEXEC)), instead >> of the if-else? > > There's a difference. set_close_on_exec() calls SetHandleInformation() > via one or more intermediate calls to fhandler_base::set_no_inheritance(). > close_on_exec() only sets the flag in the fhandler status flags. The > if-else avoids two or more unnecessary function calls in the dup/dup2 case. Is there a window here where a newly created HANDLE is inheritable even though it will later be marked cloexec? The whole point of O_CLOEXEC in POSIX is that the action is atomic, and that no other thread can ever win the race to leak the handle into an exec'd child process. Which implies that anywhere possible, we should be requesting noinherit as part of creating the HANDLE, rather than changing it after the fact. Most of your patch appeared to be doing that (with the sec_none_nih vs. sec_none conditionals), but calling set_close_on_exec seems like it is too late. >> (cmd == F_DUPFD_CLOEXEC) * O_CLOEXEC); > > Ouch, no, thanks. I'd rather ?: it. No problem - that's just style, and a good compiler will (hopefully) emit the same code for that. >> According to spec, Linux dup3(-1,-1,0) can fail with either EBADF or >> EINVAL; I haven't tested that on Linux yet, but am assuming that your >> choice of EBADF for this condition was intentional? > > Erm... EINVAL, and yes, it was intentional, per the Linux man page: > > ERRORS > [...] > EINVAL (dup3()) flags contain an invalid value. Or, oldfd was equal to > newfd. The Linux man page did not give a preference between EINVAL and EBADF. dup3(-1,-1,0) and dup3(1000000,1000000,0) are instances where both errors are equally applicable to the situation, but EINVAL is probably less computing power to determine. But your code picked different errnos for the two cases. Unfortunately, the gnulib unit test that I posted did not test those two conditions to see which errno Linux preferred (and whether it was the same errno for both calls). But since Linux is the only platform with dup3, it would be nice to guarantee the same errno for both of those calls. I guess that means I should take some time to actually compile a simple app on Linux to test that behavior. > That's very easy to answer: Because http://cygwin.com/acronyms/#SHTDI > and I have only so much time. I'm just trying to lay the groundwork > here. I'm not at all opposed to patches adding the still missing > functionality. Fair enough. -- Don't work too hard, make some time for fun as well! Eric Blake ebb9@byu.net
Attachment:
signature.asc
Description: OpenPGP digital signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |