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 05:45:36PM +0100, Corinna Vinschen wrote:
>On Jan 14 11:26, Christopher Faylor wrote:
>> On Thu, Jan 14, 2010 at 10:40:27AM +0100, Corinna Vinschen wrote:
>> >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.
>
>In my revised patch it's in fhandler_fifo.cc, called sec_user_cloexec.
>First I was planning to make a similar sec_none_cloexec function in
>fhandler.h, but it didn't make it for two reasons:
>
>- It should be rather in security.h, where sec_none and sec_none_nih
>  are declared as well.
>
>- If it's inline in security.h, it breaks the build since O_CLOEXEC
>  isn't always known in files which include security.h.
>
>Therefore it's a macro in security.h now.  The sec_user_cloexec
>doesn't get an `int flags' but a bool due to the way it gets called
>in fhandler_fifo::wait.
>
>To unify this, I could make sec_user_cloexec a macro in security.h as
>well, and change the flags parameter to a bool parameter in the
>sec_none_cloexec macro, like this:
>
>  #define sec_none_cloexec(f) (((f) ? &sec_none_nih : &sec_none))
>  #define sec_user_cloexec(f,sa,sid) (((f) ? sec_user_nih ((sa),(sid))
>					   : sec_user ((sa),(sid))))
>
>If you prefer that, I have no problem whatsoever to change it that way.

I have no preference so whatever you have is fine.

>>>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.
>
>No worries.  Parts of my patch suffered from the same problem ;)

Part of my problem is that I've recently discovered Runes of Magic and
that has been occupying way too much of my time.  But on the plus side
I'm now a Level 18 Mage with a level 14 Scout secondary so at least I
have something positive to show for my lack of sleep.

cgf


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