[PATCH] add support for POSIX_SPAWN_SETSID

[PATCH] add support for POSIX_SPAWN_SETSID

[daurnimator]

This patch adds support for the POSIX_SPAWN_SETSID flag.

It was recently accepted by the Austin Group:
http://austingroupbugs.net/view.php?id=1044

---
 include/spawn.h           | 1 +
 src/process/posix_spawn.c | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/include/spawn.h b/include/spawn.h
index 29c799ee..7dee7cfa 100644
--- a/include/spawn.h
+++ b/include/spawn.h
@@ -21,6 +21,7 @@ struct sched_param;
 #define POSIX_SPAWN_SETSIGMASK 8
 #define POSIX_SPAWN_SETSCHEDPARAM 16
 #define POSIX_SPAWN_SETSCHEDULER 32
+#define POSIX_SPAWN_SETSID 64
 
 typedef struct {
 	int __flags;
diff --git a/src/process/posix_spawn.c b/src/process/posix_spawn.c
index 0bdf71cd..03392ba8 100644
--- a/src/process/posix_spawn.c
+++ b/src/process/posix_spawn.c
@@ -73,6 +73,10 @@ static int child(void *args_vp)
 		__libc_sigaction(i, &sa, 0);
 	}
 
+	if (attr->__flags & POSIX_SPAWN_SETSID)
+		if ((ret=__syscall(SYS_setsid)))
+			goto fail;
+
 	if (attr->__flags & POSIX_SPAWN_SETPGROUP)
 		if ((ret=__syscall(SYS_setpgid, 0, attr->__pgrp)))
 			goto fail;
-- 
2.12.1

Re: [PATCH] add support for POSIX_SPAWN_SETSID

[Rich Felker]

On Fri, Mar 31, 2017 at 04:02:43PM +1100, daurnimator wrote:
> This patch adds support for the POSIX_SPAWN_SETSID flag.
> 
> It was recently accepted by the Austin Group:
> http://austingroupbugs.net/view.php?id=1044
> 
> ---
>  include/spawn.h           | 1 +
>  src/process/posix_spawn.c | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/include/spawn.h b/include/spawn.h
> index 29c799ee..7dee7cfa 100644
> --- a/include/spawn.h
> +++ b/include/spawn.h
> @@ -21,6 +21,7 @@ struct sched_param;
>  #define POSIX_SPAWN_SETSIGMASK 8
>  #define POSIX_SPAWN_SETSCHEDPARAM 16
>  #define POSIX_SPAWN_SETSCHEDULER 32
> +#define POSIX_SPAWN_SETSID 64

This overlaps with the glibc value for POSIX_SPAWN_USEVFORK; while we
don't implement it, we also don't want to have mismatched constant
ABI.

I know this is asking a lot, but could you possibly submit a glibc
patch too so the intended value (128 I guess?) is established on their
side too, or (yay if you can!) poke somebody else to do it? In the
past I've tried to just get them to casually agree to assigning values
for things like this before they implement them, but I haven't had
much luck.

Cc'ing libc-alpha too.

Rich

Re: [PATCH] add support for POSIX_SPAWN_SETSID

[Rich Felker]

On Fri, Mar 31, 2017 at 12:01:56PM -0400, Rich Felker wrote:
> On Fri, Mar 31, 2017 at 04:02:43PM +1100, daurnimator wrote:
> > This patch adds support for the POSIX_SPAWN_SETSID flag.
> > 
> > It was recently accepted by the Austin Group:
> > http://austingroupbugs.net/view.php?id=1044
> > 
> > ---
> >  include/spawn.h           | 1 +
> >  src/process/posix_spawn.c | 4 ++++
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/include/spawn.h b/include/spawn.h
> > index 29c799ee..7dee7cfa 100644
> > --- a/include/spawn.h
> > +++ b/include/spawn.h
> > @@ -21,6 +21,7 @@ struct sched_param;
> >  #define POSIX_SPAWN_SETSIGMASK 8
> >  #define POSIX_SPAWN_SETSCHEDPARAM 16
> >  #define POSIX_SPAWN_SETSCHEDULER 32
> > +#define POSIX_SPAWN_SETSID 64
> 
> This overlaps with the glibc value for POSIX_SPAWN_USEVFORK; while we
> don't implement it, we also don't want to have mismatched constant
> ABI.
> 
> I know this is asking a lot, but could you possibly submit a glibc
> patch too so the intended value (128 I guess?) is established on their
> side too, or (yay if you can!) poke somebody else to do it? In the
> past I've tried to just get them to casually agree to assigning values
> for things like this before they implement them, but I haven't had
> much luck.
> 
> Cc'ing libc-alpha too.

Thanks for all your work on this! Based on the glibc patch review, I'm
changing the error condition from !=0 to <0, and of course changing
the flag value to 128 to match.

Rich

Re: [musl] [PATCH] add support for POSIX_SPAWN_SETSID

[Zack Weinberg]

On Sat, Apr 22, 2017 at 7:19 PM, Rich Felker <dalias@libc.org> wrote:
>> >  #define POSIX_SPAWN_SETSIGMASK 8
>> >  #define POSIX_SPAWN_SETSCHEDPARAM 16
>> >  #define POSIX_SPAWN_SETSCHEDULER 32
>> > +#define POSIX_SPAWN_SETSID 64
>>
>> This overlaps with the glibc value for POSIX_SPAWN_USEVFORK; while we
>> don't implement it, we also don't want to have mismatched constant
>> ABI.
...
> Thanks for all your work on this! Based on the glibc patch review, I'm
> changing the error condition from !=0 to <0, and of course changing
> the flag value to 128 to match.

May I suggest that musl include a note in its headers, explaining that
bit 64 is reserved to avoid an ABI conflict with glibc?  Then the next
time there's an addition, there will be less confusion.

zw

Re: [PATCH] add support for POSIX_SPAWN_SETSID

[Rich Felker]

On Sat, Apr 22, 2017 at 07:43:01PM -0400, Zack Weinberg wrote:
> On Sat, Apr 22, 2017 at 7:19 PM, Rich Felker <dalias@libc.org> wrote:
> >> >  #define POSIX_SPAWN_SETSIGMASK 8
> >> >  #define POSIX_SPAWN_SETSCHEDPARAM 16
> >> >  #define POSIX_SPAWN_SETSCHEDULER 32
> >> > +#define POSIX_SPAWN_SETSID 64
> >>
> >> This overlaps with the glibc value for POSIX_SPAWN_USEVFORK; while we
> >> don't implement it, we also don't want to have mismatched constant
> >> ABI.
> ....
> > Thanks for all your work on this! Based on the glibc patch review, I'm
> > changing the error condition from !=0 to <0, and of course changing
> > the flag value to 128 to match.
> 
> May I suggest that musl include a note in its headers, explaining that
> bit 64 is reserved to avoid an ABI conflict with glibc?  Then the next
> time there's an addition, there will be less confusion.

I was actually thinking of just adding POSIX_SPAWN_USEVFORK as a nop.
I think it will eventually be a nop on glibc too; maybe it already is
on Linux targets. Then we can have posix_spawnattr_setflags check
against all valid flags and return EINVAL if an unknown bit is set, so
that applications can runtime-probe for any future functionality
additions.

Rich

Re: [musl] [PATCH] add support for POSIX_SPAWN_SETSID

[Zack Weinberg]

On Sat, Apr 22, 2017 at 7:47 PM, Rich Felker <dalias@libc.org> wrote:
> On Sat, Apr 22, 2017 at 07:43:01PM -0400, Zack Weinberg wrote:
>> On Sat, Apr 22, 2017 at 7:19 PM, Rich Felker <dalias@libc.org> wrote:
>> >> >  #define POSIX_SPAWN_SETSIGMASK 8
>> >> >  #define POSIX_SPAWN_SETSCHEDPARAM 16
>> >> >  #define POSIX_SPAWN_SETSCHEDULER 32
>> >> > +#define POSIX_SPAWN_SETSID 64
>> >>
>> >> This overlaps with the glibc value for POSIX_SPAWN_USEVFORK; while we
>> >> don't implement it, we also don't want to have mismatched constant
>> >> ABI.
>> ....
>> > Thanks for all your work on this! Based on the glibc patch review, I'm
>> > changing the error condition from !=0 to <0, and of course changing
>> > the flag value to 128 to match.
>>
>> May I suggest that musl include a note in its headers, explaining that
>> bit 64 is reserved to avoid an ABI conflict with glibc?  Then the next
>> time there's an addition, there will be less confusion.
>
> I was actually thinking of just adding POSIX_SPAWN_USEVFORK as a nop.
> I think it will eventually be a nop on glibc too; maybe it already is
> on Linux targets. Then we can have posix_spawnattr_setflags check
> against all valid flags and return EINVAL if an unknown bit is set, so
> that applications can runtime-probe for any future functionality
> additions.

Makes sense.

zw