[PATCH v3] posix: spawn: Make handling of invalid fds consistent
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Mon Mar 30 13:51:08 GMT 2026
On 26/03/26 11:13, Lucas Chollet wrote:
> in `posix_spawn_file_actions_*` functions.
>
> As mandated by POSIX, these functions should return `EBADF` on negative
> file descriptors but let everything else fail during the call to
> `posix_spawn`.
>
> Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
Hi Lucas, thanks again for the patch. It looks good, but I think it would
be better to wait for Austin remark on Collin's defect [1]. I think we
can wait until the release.
[1] https://austingroupbugs.net/view.php?id=1977
> ---
> v3: Make all spawn_faction routines consistent with regards to invalid
> fd handling.
>
> posix/Makefile | 1 -
> posix/spawn_faction_addclose.c | 2 +-
> posix/spawn_faction_addclosefrom.c | 2 +-
> posix/spawn_faction_adddup2.c | 2 +-
> posix/spawn_faction_addfchdir.c | 3 +++
> posix/spawn_faction_addopen.c | 2 +-
> posix/spawn_faction_addtcsetpgrp_np.c | 2 +-
> posix/spawn_int.h | 4 ----
> posix/spawn_valid_fd.c | 30 ---------------------------
> posix/tst-posix_spawn-fd.c | 2 +-
> 10 files changed, 9 insertions(+), 41 deletions(-)
> delete mode 100644 posix/spawn_valid_fd.c
>
> diff --git a/posix/Makefile b/posix/Makefile
> index ec28b9e1da..bf5d5a8e23 100644
> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -155,7 +155,6 @@ routines := \
> spawn_faction_addtcsetpgrp_np \
> spawn_faction_destroy \
> spawn_faction_init \
> - spawn_valid_fd \
> spawnattr_destroy \
> spawnattr_getdefault \
> spawnattr_getflags \
> diff --git a/posix/spawn_faction_addclose.c b/posix/spawn_faction_addclose.c
> index 3073c05761..5a0dc6c1cf 100644
> --- a/posix/spawn_faction_addclose.c
> +++ b/posix/spawn_faction_addclose.c
> @@ -29,7 +29,7 @@ __posix_spawn_file_actions_addclose (posix_spawn_file_actions_t *file_actions,
> {
> struct __spawn_action *rec;
>
> - if (!__spawn_valid_fd (fd))
> + if (fd < 0)
> return EBADF;
>
> /* Allocate more memory if needed. */
> diff --git a/posix/spawn_faction_addclosefrom.c b/posix/spawn_faction_addclosefrom.c
> index 5c56d035c6..f55d69b2ea 100644
> --- a/posix/spawn_faction_addclosefrom.c
> +++ b/posix/spawn_faction_addclosefrom.c
> @@ -28,7 +28,7 @@ __posix_spawn_file_actions_addclosefrom (posix_spawn_file_actions_t
> #if __SPAWN_SUPPORT_CLOSEFROM
> struct __spawn_action *rec;
>
> - if (!__spawn_valid_fd (from))
> + if (from < 0)
> return EBADF;
>
> /* Allocate more memory if needed. */
> diff --git a/posix/spawn_faction_adddup2.c b/posix/spawn_faction_adddup2.c
> index f9564ef4c2..9190eb2a75 100644
> --- a/posix/spawn_faction_adddup2.c
> +++ b/posix/spawn_faction_adddup2.c
> @@ -29,7 +29,7 @@ __posix_spawn_file_actions_adddup2 (posix_spawn_file_actions_t *file_actions,
> {
> struct __spawn_action *rec;
>
> - if (!__spawn_valid_fd (fd) || !__spawn_valid_fd (newfd))
> + if (fd < 0 || newfd < 0)
> return EBADF;
>
> /* Allocate more memory if needed. */
> diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
> index a3df6df489..e851af1722 100644
> --- a/posix/spawn_faction_addfchdir.c
> +++ b/posix/spawn_faction_addfchdir.c
> @@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
> {
> struct __spawn_action *rec;
>
> + if (fd < 0)
> + return EBADF;
> +
> /* Allocate more memory if needed. */
> if (actions->__used == actions->__allocated
> && __posix_spawn_file_actions_realloc (actions) != 0)
> diff --git a/posix/spawn_faction_addopen.c b/posix/spawn_faction_addopen.c
> index bb9fc41b99..bd610e79b0 100644
> --- a/posix/spawn_faction_addopen.c
> +++ b/posix/spawn_faction_addopen.c
> @@ -31,7 +31,7 @@ __posix_spawn_file_actions_addopen (posix_spawn_file_actions_t *file_actions,
> {
> struct __spawn_action *rec;
>
> - if (!__spawn_valid_fd (fd))
> + if (fd < 0)
> return EBADF;
>
> char *path_copy = __strdup (path);
> diff --git a/posix/spawn_faction_addtcsetpgrp_np.c b/posix/spawn_faction_addtcsetpgrp_np.c
> index 291b115212..6f2f0d315e 100644
> --- a/posix/spawn_faction_addtcsetpgrp_np.c
> +++ b/posix/spawn_faction_addtcsetpgrp_np.c
> @@ -27,7 +27,7 @@ __posix_spawn_file_actions_addtcsetpgrp_np (posix_spawn_file_actions_t
> {
> struct __spawn_action *rec;
>
> - if (!__spawn_valid_fd (tcfd))
> + if (tcfd < 0)
> return EBADF;
>
> /* Allocate more memory if needed. */
> diff --git a/posix/spawn_int.h b/posix/spawn_int.h
> index 0291526edb..297a203c65 100644
> --- a/posix/spawn_int.h
> +++ b/posix/spawn_int.h
> @@ -87,8 +87,4 @@ extern int __spawni (int *pid, const char *path,
> const posix_spawnattr_t *attrp, char *const argv[],
> char *const envp[], int xflags) attribute_hidden;
>
> -/* Return true if FD falls into the range valid for file descriptors.
> - The check in this form is mandated by POSIX. */
> -bool __spawn_valid_fd (int fd) attribute_hidden;
> -
> #endif /* _SPAWN_INT_H */
> diff --git a/posix/spawn_valid_fd.c b/posix/spawn_valid_fd.c
> deleted file mode 100644
> index 1d4cd99256..0000000000
> --- a/posix/spawn_valid_fd.c
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -/* File descriptor validity check for posix_spawn file actions.
> - Copyright (C) 2000-2026 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <https://www.gnu.org/licenses/>. */
> -
> -#include "spawn_int.h"
> -
> -#include <unistd.h>
> -
> -bool
> -__spawn_valid_fd (int fd)
> -{
> - long maxfd = __sysconf (_SC_OPEN_MAX);
> - return __glibc_likely (fd >= 0)
> - && (__glibc_unlikely (maxfd < 0) /* No limit set. */
> - || __glibc_likely (fd < maxfd));
> -}
> diff --git a/posix/tst-posix_spawn-fd.c b/posix/tst-posix_spawn-fd.c
> index e75d829562..72a40ff9a3 100644
> --- a/posix/tst-posix_spawn-fd.c
> +++ b/posix/tst-posix_spawn-fd.c
> @@ -72,7 +72,7 @@ all_tests (const char *name, int (*func) (int))
> one_test (name, func, -1, false);
> one_test (name, func, -2, false);
> if (maxfd >= 0)
> - one_test (name, func, maxfd, false);
> + one_test (name, func, maxfd, true);
> }
>
> static int
More information about the Libc-alpha
mailing list