[Patch]: ciresrv.parent

Pierre A. Humblet pierre@phumblet.no-ip.org
Mon Feb 2 14:47:00 GMT 2004



Corinna Vinschen wrote:
> 
> On Feb  1 16:57, Pierre A. Humblet wrote:
> > At 01:39 PM 2/1/2004 -0500, Christopher Faylor wrote:
> > >On Sat, Jan 31, 2004 at 02:18:48PM -0500, Pierre A. Humblet wrote:
> > >>Fortunately it is never used in the case of spawn: all handles are
> > >>inherited, or the parent does the work (sockets).
> > >
> > >The one placed the handle is actually used is in
> > >fhandler_socket::fixup_after_exec.  I'd like Corinna's confirmation
> > >before this patch is checked in.
> >
> > Good idea. FWIW, I checked that one carefully. That's why I found
> > the secret_event bug a while back. I also tested on Win95 with an
> > old winsock.
> > It looks like the handle might be used, but the tests for close
> > on exec always block the paths where it is actually used.
> 
> AFAICS, you're right.  fhandler_socket::fixup_after_exec calls
> fhandler_socket::fixup_after_fork only if !close_on_exec.
> fhandler_socket::fixup_after_fork in turn calls fork_fixup which only
> uses the parent handle if close_on_exec.  So the parent handle is never
> used in this scenario.  So I think it's ok to drop the parent handle.

Do you want to apply the patch now, or I do it this evening?

To simplify the life of future maintainers, I was thinking of
redoing fixup_after_exec as follows
(instead of going through the cascade of calls above)

void
fhandler_socket::fixup_after_exec ()
{
  SOCKET new_sock = WSASocketA (FROM_PROTOCOL_INFO,
				FROM_PROTOCOL_INFO,
				FROM_PROTOCOL_INFO,
				prot_info_ptr, 0, 0);
  debug_printf("%x = WSASocketA(FROM_PROTOCOL_INFO)", new_sock);
  if (new_sock) /* winsock2 */
    set_io_handle ((HANDLE) new_sock);
}
 
> As a side note, it took me a while to understand that it's the same
> situation for the secret_event handle.  The problem is the name of the
> function set_inheritance().  The second parameter is the *negation*
> of the inheritance.  IMHO this is rather confusing.  Either we should
> rename the function to set_no_inheritance or we should revert the
> meaning of the second parameter.

I am all for that! Renaming the function has my preference.

Pierre



More information about the Cygwin-patches mailing list