[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: fix mkostemp, add pipe2-safer

From: Eric Blake
Subject: Re: fix mkostemp, add pipe2-safer
Date: Sun, 06 Dec 2009 17:53:03 -0700
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20090812 Thunderbird/ Mnenhy/

Hash: SHA1

According to Bruno Haible on 12/6/2009 5:18 PM:
> - In set_cloexec_flag: You added a call to
>     dup2 (desc, desc)
>   But on a POSIX compliant system this is a no-op, since

Not quite.  It is the fastest one-syscall sequence for determining whether
an fd is open (if it returns -1, then errno is properly set to EBADF; if
it returns desc [and desc was not -1], then the fd is valid).  Other
options, like fcntl(F_GETFL), are not as portable.  We've used the same
paradigm in other gnulib files (such as utimens.c).  But yes, it does
require the use of the gnulib dup2 module to work around dup2 bugs.

>     <http://www.opengroup.org/onlinepubs/9699919799/functions/dup.html>
>   says:
>      "If fildes is equal to fildes2, the FD_CLOEXEC flag associated with
>       fildes2 shall not be changed."
>   If this is specifically targeted at non-POSIX systems, it would be good to 
> have
>   a comment about it.

Okay, then, a comment describing that dup2 is used as an EBADF filter, and
not for any side effects on cloexec (since there aren't any side effects),
would make sense.

> - Regarding dup_safer_flag and fd_safer_flag: I find it important to also 
> handle
>   also O_BINARY and O_TEXT, like we do in the pipe2 function (see the doc in
>   lib/unistd.in.h). pipe2 supports O_TEXT, but pipe2_safer does not: It
>   respects the O_TEXT flag in the call to pipe2 but then loses it in the call 
> to
>   fd_safer_flag (because dup_cloexec has an O_BINARY flag hardwired).

Good point.  I see two options:

Change dup_cloexec to take a flag, so that mingw can handle text vs.
binary during the dup.  For cygwin, it would have to call setmode() after
the fact.

Leave dup_cloexec as is, and make fd_safer_flag and/or dup_safer_flag call
setmode() after the fact for both cygwin and mingw.

I'm leaning quite heavily to the latter, particularly since dup_cloexec is
a nice shoe-in for fcntl(F_DUPFD_CLOEXEC), which also lacks the flag for
setting text vs. binary.  So I'll see about writing such a patch, if you
don't beat me to it.

>   I'm not sure about the way to get it: Should we add O_CLOEXEC support for 
> all
>   open-like functions, defining O_CLOEXEC ourselves? Or should we add a
>   O_GL_CLOEXEC that is supported only by selected gnulib functions? The latter
>   approach can serve as a step-by-step approach to the complete support of

Well, we've been talking for a couple of months about implementing
O_CLOEXEC in gnulib.  I'm not sure whether (temporarily) using
O_GL_CLOEXEC is going to get us there any faster than just doing the work.

> 2009-12-06  Bruno Haible  <address@hidden>
>       * lib/cloexec.c (dup_cloexec): Fix handling of _gl_register_dup return
>       value.

Thanks for catching that.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/


reply via email to

[Prev in Thread] Current Thread [Next in Thread]