Re: Fixing posix_spawn_file_actions_addopen with O_CLOEXEC

On Jul  8 10:38, Jeremy Drake wrote:
> (continuation from cygwin-patches thread)
> 
> On Tue, 8 Jul 2025, Corinna Vinschen wrote:
> 
> > On Jul  7 14:43, Jeremy Drake via Cygwin-patches wrote:
> > > I noticed a bit that you quoted that I hadn't noticed before
> > > additionally, when the new process image is executed, any
> > >   file descriptor (from this new set) which has its FD_CLOEXEC flag set
> > >   shall be closed (see posix_spawn()).
> > >
> > > The "from this new set" is not handled properly in my implementation, and
> > > I'm pretty sure not from the existing newlib implementation: I copied what
> > > it was doing, which was to clear the FD_SETFD flag after open.  That
> > > would be wrong, if faced with an addopen with the O_CLOEXEC flag set.
> > > This is obviously a stupid thing for a caller to do...
> >
> > the POSIX posix_spawn man page has a 4 steps list how to process
> > the file actions with step 4: close all FD_CLOEXEC descriptors.
> >
> > I wonder if that doesn't make some sense, e.g.
> >
> >   addopen (&fact, 42, "somedir", O_CLOEXEC);
> >   addfchdir (42);
> >
> > but then again, you could just add an addclose(42) and you would have the
> > same effect.
> >
> > Well, *shrug*, as long as we can do it right.
> >
> > But yeah, I don't see anywhere in the POSIX docs that the addopen
> > descriptors have the FD_CLOEXEC flag removed automatically.  They
> > are supposed to be closed instead, FWIW.
> >
> > A fix to newlib's posix_spawn might make sense.
> 
> OK.  I'm not sure about what functions are allowed to  be used there,
> though (fcntl is in a preprocessor check, so I guess not all posix funcs
> can be assumed to be present).  Can I use dup3 or do I need to fcntl
> F_SETFD after the dup2?  Should I use the O_CLOEXEC flag to determine this
> or use F_GETFD on the open-ed fd to F_SETFD on the dup2-ed one?

I'm not sure what you're trying to accomplish.  Given that we execve()
from here, step 4 from the POSIX.1-2024 man page is basically implied.

So afaics, the only necessary patch is dropping the _fcntl call from
the FAE_OPEN case, no?

> Also, I noticed a new spawnattr flag for calling setsid () in the child, I
> could do a patch to add that, but is that function allowed to be used or
> does it need a preprocessor check?

I don't think a check is necessary.  Just add setsid to the list
of "Supporting OS subroutines required" in the file header.


Thanks,
Corinna