[PATCH 3/3] posix: Add posix_spawn_file_actions_closefrom

[Florian Weimer]

* Adhemerval Zanella:

>>> diff --git a/posix/spawn.h b/posix/spawn.h
>>> index be6bd591a3..21ea563425 100644
>>> --- a/posix/spawn.h
>>> +++ b/posix/spawn.h
>>> @@ -213,6 +213,13 @@ extern int posix_spawn_file_actions_addchdir_np (posix_spawn_file_actions_t *
>>>  extern int posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *,
>>>  						  int __fd)
>>>       __THROW __nonnull ((1));
>>> +
>>> +/* Add an action to close all file descriptor greater than FROM during
>>> +   spawn.  This affects the subsequent file actions.  */
>>> +extern int posix_spawn_file_actions_addclosefrom_np (posix_spawn_file_actions_t *,
>>> +						     int __from)
>>> +     __THROW __nonnull ((1));
>>> +
>>>  #endif
>> 
>> Line length issue (but I'm not sure how to avoid that).
>
> Maybe move the argument to next line aligning with __THROW?
>
> extern int posix_spawn_file_actions_addclosefrom_np (
>      posix_spawn_file_actions_t *, int __from)
>      __THROW __nonnull ((1));
>
> ?

I think it's not GNU style.  You could drop the extern, though.

>> You could simplify the reinvocation logic by making this a static or a
>> container test.
>
> I prefer to keep this exercises this test on some platform where container
> tests are not really supported.

Then perhaps make it a static test, or only make it a container test for
the non-hard-coded-paths case?

>>> diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
>>> index f157bfffd2..f496578d19 100644
>>> --- a/sysdeps/unix/sysv/linux/spawni.c
>>> +++ b/sysdeps/unix/sysv/linux/spawni.c
>>> @@ -16,22 +16,17 @@
>>>     License along with the GNU C Library; if not, see
>>>     <https://www.gnu.org/licenses/>.  */
>>>  
>>> -#include <spawn.h>
>>> -#include <fcntl.h>
>>> -#include <paths.h>
>>> -#include <string.h>
>>> -#include <sys/resource.h>
>>> -#include <sys/wait.h>
>>> -#include <sys/param.h>
>>> -#include <sys/mman.h>
>>> -#include <not-cancel.h>
>>> +#include <arch-fd_to_filename.h>
>>> +#include <internal-signals.h>
>>> +#include <ldsodefs.h>
>>>  #include <local-setxid.h>
>>> +#include <not-cancel.h>
>>> +#include <paths.h>
>>>  #include <shlib-compat.h>
>>> -#include <nptl/pthreadP.h>
>>> -#include <dl-sysdep.h>
>>> -#include <libc-pointer-arith.h>
>>> -#include <ldsodefs.h>
>>> -#include "spawn_int.h"
>>> +#include <spawn.h>
>>> +#include <spawn_int.h>
>>> +#include <sysdep.h>
>>> +#include <sys/resource.h>
>> 
>> Suprious changes?
>
> In fact I cleanup the include range a bit, if you prefer I can remove this
> change from this set.

Hmm.  I think <arch-fd_to_filename.h> is not used here?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill