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]

Re: dup3/O_CLOEXEC/F_DUPFD_CLOEXEC


On Thu, Jan 14, 2010 at 10:40:27AM +0100, Corinna Vinschen wrote:
>On Jan 13 16:49, Christopher Faylor wrote:
>> On Wed, Jan 13, 2010 at 10:25:37PM +0100, Corinna Vinschen wrote:
>> >Hi,
>> >
>> >the below patch implements the Linux dup3/O_CLOEXEC/F_DUPFD_CLOEXEC
>> >extension.  I hope I didn't miss anything important since it affects
>> >quite a few fhandlers.  Fortunately most is mechanical change, except
>> >for a few places (dtable.cc, pipe.cc, fhandeler_fifo.cc, syscalls.cc).
>> >Nevertheless, I'd be glad if somebody could have a second look into
>> >this.
>> >
>> >Eric, you asked for it in the first place, do you have a fine testcase
>> >for this functionality?
>> 
>> The number of times that you typed:
>> 
>>   sa_buf = close_on_exec ()
>>               ? sec_user_nih ((PSECURITY_ATTRIBUTES) char_sa_buf, cygheap->user.sid())
>>               : sec_user ((PSECURITY_ATTRIBUTES) char_sa_buf, cygheap->user.sid());
>> 
>> implies that this should be a macro or a function.
>
>The combination with sec_none_nih/sec_none is used four times in
>different fhandler files.  Yes, good idea, I'll create an inline
>function in fhandler.h.
>
>The above combination with sec_user_nih/sec_user is only used two times,
>both in fhandler_fifo.cc.  What about an inline function in
>fhandler_fifo.cc for this one?  I'll add that to the revised patch.

Even though it's used in fhandler_fifo is it similar enough to the
other function that grouping them together might make things clearer?
Otherwise, nevermind.

>> Could the setting of close_on_exec be handled in the syscalls.cc open()
>> so that it doesn't have to be done so many times?  You could have
>
>Yesterday I was sure that it has to be set in the various open methods
>since they could be called from elsewhere.  Today, after a nights sleep,
>I'm not so sure anymore.  I don't see any call to fh->open outside of
>open(2).  And calls to the open_fs() function are covered anyway.
>I'll look into simplifying this.
>
>> build_fh_name set the noexec flag so that close_on_exec() would still
>> work in the fhandler_*::open functions.
>
>I'm not sure I understand you correctly.  Do you mean build_fh_name
>should already set the close_on_exec flag so that later fhandler_*::open
>only have to call set_close_on_exec if a set_no_inheritance call is
>required?

I was saying that build_fh_name could set the flag so that you could
query close_on_exec() rather than using "flags & O_CLOEXEC".  But I made
that suggestion too late in the night and I see now that the flags parameter
that I thought was being passed to build_fh_name is not that at all.  So:

>For now I'll go the road to add the default close_on_exec setting to the
>open(2) call.  It's easy to switch to build_fh_name from there.

I don't think we need to perturb build_fh_name.  It was just a sleep-addled
suggestion.

cgf


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]