Re: posix_spawn_file_actions_addchdir/addfchdir

Daniel Dickman <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
Looks mostly okay, passes sortix os-tests for what it's worth.

A few tweaks inline below.

For the code:
- add POSIX_VISIBLE guards
- remove extra "int errors"
- remove extra "__restrict"

For the man page see comments below.


On Thu, 6 Aug 2026, Stuart Henderson wrote:

> posix_spawn_file_actions_addchdir and posix_spawn_file_actions_addfchdir
> have been implemented as _np in some OS for a while, but have made it
> into posix now.
> 
> https://pubs.opengroup.org/onlinepubs/9799919799.2024edition/functions/posix_spawn_file_actions_addchdir.html
> 
> a number of things in ports print information about testing for them
> during configure (mostly via gnulib which provides replacements).
> also the subprocess mode in llama.cpp is forcibly disabled via a
> patch because we don't have ..._addchdir().
> 
> https://exopi.bsdfrog.org/cgi-bin/omega?P=posix_spawn_file_actions_addchdir&DEFAULTOP=and&DB=default&FMT=query&xDB=default&xFILTERS=5
> 
> do we want them (yet)? is this diff vaguely along the right lines?
> (obviously needs shlib bump and testing in ports bulk).
> 
> I wrote an initial diff, then discovered FreeBSD had them already
> (then added the missing free which I spotted after comparing ;)
> 
> Index: include/spawn.h
> ===================================================================
> RCS file: /cvs/src/include/spawn.h,v
> diff -u -p -r1.3 spawn.h
> --- include/spawn.h	20 May 2015 22:50:07 -0000	1.3
> +++ include/spawn.h	6 Aug 2026 12:18:31 -0000
> @@ -69,6 +69,9 @@ int posix_spawn_file_actions_addopen(pos
>      int, const char *__restrict, int, mode_t);
>  int posix_spawn_file_actions_adddup2(posix_spawn_file_actions_t *, int, int);
>  int posix_spawn_file_actions_addclose(posix_spawn_file_actions_t *, int);
> +int posix_spawn_file_actions_addchdir(posix_spawn_file_actions_t *__restrict,
> +    const char *__restrict);
> +int posix_spawn_file_actions_addfchdir(posix_spawn_file_actions_t *, int);


Should have POSIX_VISIBLE guards for 2024. We can look to add SETSID 
support separately and I don't think that's needed as part of this diff.


>  
>  /*
>   * Spawn attributes
> Index: lib/libc/Symbols.list
> ===================================================================
> RCS file: /cvs/src/lib/libc/Symbols.list,v
> diff -u -p -r1.95 Symbols.list
> --- lib/libc/Symbols.list	24 Oct 2025 11:30:06 -0000	1.95
> +++ lib/libc/Symbols.list	6 Aug 2026 12:18:31 -0000
> @@ -725,8 +725,10 @@ pause
>  pclose
>  popen
>  posix_spawn
> +posix_spawn_file_actions_addchdir
>  posix_spawn_file_actions_addclose
>  posix_spawn_file_actions_adddup2
> +posix_spawn_file_actions_addfchdir
>  posix_spawn_file_actions_addopen
>  posix_spawn_file_actions_destroy
>  posix_spawn_file_actions_init
> Index: lib/libc/gen/posix_spawn.3
> ===================================================================
> RCS file: /cvs/src/lib/libc/gen/posix_spawn.3,v
> diff -u -p -r1.11 posix_spawn.3
> --- lib/libc/gen/posix_spawn.3	26 Jun 2023 15:28:52 -0000	1.11
> +++ lib/libc/gen/posix_spawn.3	6 Aug 2026 12:18:31 -0000
> @@ -73,8 +73,10 @@ with
>  Otherwise, file descriptors in the child process
>  are altered according to
>  .Xr posix_spawn_file_actions_init 3 ,
> +.Xr posix_spawn_file_actions_addchdir 3 ,
>  .Xr posix_spawn_file_actions_addclose 3 ,
>  .Xr posix_spawn_file_actions_adddup2 3 ,
> +.Xr posix_spawn_file_actions_addfchdir 3 ,
>  and
>  .Xr posix_spawn_file_actions_addopen 3 .
>  .Pp
> Index: lib/libc/gen/posix_spawn.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/gen/posix_spawn.c,v
> diff -u -p -r1.10 posix_spawn.c
> --- lib/libc/gen/posix_spawn.c	28 Jun 2019 13:32:41 -0000	1.10
> +++ lib/libc/gen/posix_spawn.c	6 Aug 2026 12:18:31 -0000
> @@ -51,7 +51,7 @@ struct __posix_spawn_file_actions {
>  
>  typedef struct __posix_spawn_file_actions_entry {
>  	SIMPLEQ_ENTRY(__posix_spawn_file_actions_entry) fae_list;
> -	enum { FAE_OPEN, FAE_DUP2, FAE_CLOSE } fae_action;
> +	enum { FAE_OPEN, FAE_DUP2, FAE_CLOSE, FAE_CHDIR, FAE_FCHDIR } fae_action;
>  
>  	int fae_fildes;
>  	union {
> @@ -171,6 +171,14 @@ process_file_actions_entry(posix_spawn_f
>  		/* Perform a close(), do not fail if already closed */
>  		(void)close(fae->fae_fildes);
>  		break;
> +	case FAE_CHDIR:
> +		if (chdir(fae->fae_path) == -1)
> +			return (errno);
> +		break;
> +	case FAE_FCHDIR:
> +		if (fchdir(fae->fae_fildes) == -1)
> +			return (errno);
> +		break;
>  	}
>  	return (0);
>  }
> @@ -273,7 +281,8 @@ posix_spawn_file_actions_destroy(posix_s
>  		SIMPLEQ_REMOVE_HEAD(&(*fa)->fa_list, fae_list);
>  
>  		/* Deallocate file action entry */
> -		if (fae->fae_action == FAE_OPEN)
> +		if (fae->fae_action == FAE_OPEN ||
> +		    fae->fae_action == FAE_CHDIR)
>  			free(fae->fae_path);
>  		free(fae);
>  	}
> @@ -352,6 +361,54 @@ posix_spawn_file_actions_addclose(posix_
>  
>  	/* Set values and store in queue */
>  	fae->fae_action = FAE_CLOSE;
> +	fae->fae_fildes = fildes;
> +
> +	SIMPLEQ_INSERT_TAIL(&(*fa)->fa_list, fae, fae_list);
> +	return (0);
> +}
> +
> +int
> +posix_spawn_file_actions_addchdir(posix_spawn_file_actions_t *__restrict fa,
> +    const char *__restrict path)
> +{
> +	posix_spawn_file_actions_entry_t *fae;
> +	int error;
> +
> +	/* Allocate object */
> +	fae = malloc(sizeof(posix_spawn_file_actions_entry_t));
> +	if (fae == NULL)
> +		return (errno);
> +
> +	/* Set values and store in queue */
> +	fae->fae_action = FAE_CHDIR;
> +	fae->fae_path = strdup(path);
> +	if (fae->fae_path == NULL) {
> +		error = errno;
> +		free(fae);
> +		return (error);
> +	}
> +
> +	SIMPLEQ_INSERT_TAIL(&(*fa)->fa_list, fae, fae_list);
> +	return (0);
> +}
> +
> +int
> +posix_spawn_file_actions_addfchdir(posix_spawn_file_actions_t *__restrict fa,
> +    int fildes)

__restrict here seems inconsistent with the header? Remove it?

> +{
> +	posix_spawn_file_actions_entry_t *fae;
> +	int error;

"int error" doesn't seem to be used? should it be removed?

> +
> +	if (fildes < 0)
> +		return (EBADF);
> +
> +	/* Allocate object */
> +	fae = malloc(sizeof(posix_spawn_file_actions_entry_t));
> +	if (fae == NULL)
> +		return (errno);
> +
> +	/* Set values and store in queue */
> +	fae->fae_action = FAE_FCHDIR;
>  	fae->fae_fildes = fildes;
>  
>  	SIMPLEQ_INSERT_TAIL(&(*fa)->fa_list, fae, fae_list);
> Index: lib/libc/gen/posix_spawn_file_actions_addopen.3
> ===================================================================
> RCS file: /cvs/src/lib/libc/gen/posix_spawn_file_actions_addopen.3,v
> diff -u -p -r1.9 posix_spawn_file_actions_addopen.3
> --- lib/libc/gen/posix_spawn_file_actions_addopen.3	29 Mar 2022 18:15:52 -0000	1.9
> +++ lib/libc/gen/posix_spawn_file_actions_addopen.3	6 Aug 2026 12:18:31 -0000
> @@ -18,17 +18,23 @@
>  .Dt POSIX_SPAWN_FILE_ACTIONS_ADDOPEN 3
>  .Os
>  .Sh NAME
> +.Nm posix_spawn_file_actions_addchdir ,
>  .Nm posix_spawn_file_actions_addclose ,
>  .Nm posix_spawn_file_actions_adddup2 ,
> +.Nm posix_spawn_file_actions_addfchdir ,
>  .Nm posix_spawn_file_actions_addopen
>  .Nd add action to close, dup2 or open file descriptor to file actions object

.Nd should be updated for the new functions

>  .Sh SYNOPSIS
>  .In spawn.h
>  .Ft int
> +.Fn posix_spawn_file_actions_addchdir "posix_spawn_file_actions_t *file_actions" "const char *restrict path"
> +.Ft int
>  .Fn posix_spawn_file_actions_addclose "posix_spawn_file_actions_t *file_actions" "int fildes"
>  .Ft int
>  .Fn posix_spawn_file_actions_adddup2 "posix_spawn_file_actions_t *file_actions" "int fildes" "int newfildes"
>  .Ft int
> +.Fn posix_spawn_file_actions_addfchdir "posix_spawn_file_actions_t *file_actions" "int fildes"
> +.Ft int
>  .Fn posix_spawn_file_actions_addopen "posix_spawn_file_actions_t *file_actions" "int fildes" "const char *restrict path" "int oflag" "mode_t mode"
>  .Sh DESCRIPTION
>  These function add an action to


While touching this file "These function" --> "These functions"

Also do we want to update DESCRIPTION for the new functions?

> @@ -45,6 +51,15 @@ Actions are executed in order in the chi
>  .Bl -dash
>  .It
>  The
> +.Fn posix_spawn_file_actions_addchdir
> +function adds an action that causes
> +.Bd -literal -offset indent
> +chdir(path);
> +.Ed
> +.Pp
> +to be called.
> +.It
> +The
>  .Fn posix_spawn_file_actions_addclose
>  function adds an action that causes
>  .Bd -literal -offset indent
> @@ -69,6 +84,15 @@ equals
>  .Fa fildes .
>  .It
>  The
> +.Fn posix_spawn_file_actions_addfchdir
> +function adds an action that causes
> +.Bd -literal -offset indent
> +fchdir(fildes);
> +.Ed
> +.Pp
> +to be called.
> +.It
> +The
>  .Fn posix_spawn_file_actions_addopen
>  function adds an action that causes
>  .Bd -literal -offset indent
> @@ -100,7 +124,12 @@ if they run out of memory.
>  .Xr posix_spawn_file_actions_init 3 ,
>  .Xr posix_spawnp 3
>  .Sh STANDARDS
> -These functions conform to
> +.Fn posix_spawn_file_actions_addchdir
> +and
> +.Fn posix_spawn_file_actions_addfchdir
> +conform to
> +.St -p1003.1-2024 .
> +Other functions conform to


I think this should say we conform to posix 2024, and not separate out the 
functions. I did a quick pass through and it should be fine to make that 
claim.

The return section is wrong (this is pre-existing your diff). Negative 
file descriptors are EBADF, not EINVAL. So you should change that at the 
same time to match what the new functions do. And that will fix things for 
the existing functions as well.

EINVAL checks could probably be added, but that is aspirational code and 
separate from your diff and is optional from my read of posix.


>  .St -p1003.1-2001 .
>  .Sh AUTHORS
>  .An \&Ed Schouten Aq Mt [email protected]
> Index: lib/libc/hidden/spawn.h
> ===================================================================
> RCS file: /cvs/src/lib/libc/hidden/spawn.h,v
> diff -u -p -r1.1 spawn.h
> --- lib/libc/hidden/spawn.h	4 Oct 2015 07:57:21 -0000	1.1
> +++ lib/libc/hidden/spawn.h	6 Aug 2026 12:18:31 -0000
> @@ -21,8 +21,10 @@
>  #include_next <spawn.h>
>  
>  PROTO_DEPRECATED(posix_spawn);
> +PROTO_DEPRECATED(posix_spawn_file_actions_addchdir);
>  PROTO_DEPRECATED(posix_spawn_file_actions_addclose);
>  PROTO_DEPRECATED(posix_spawn_file_actions_adddup2);
> +PROTO_DEPRECATED(posix_spawn_file_actions_addfchdir);
>  PROTO_DEPRECATED(posix_spawn_file_actions_addopen);
>  PROTO_DEPRECATED(posix_spawn_file_actions_destroy);
>  PROTO_DEPRECATED(posix_spawn_file_actions_init);
> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.