Re: [musl] Re: [PATCH] C23: update some legacy function pointers

[Rich Felker <dalias@libc.org>]

On Mon, Apr 15, 2024 at 08:57:35PM -0700, Michael Forney wrote:
> Jens Gustedt <Jens.Gustedt@inria.fr> wrote:
> > In C23, empty parameter lists loose their meaning as "function that
> > may receive any number of parameters".
> > 
> > When compiling with -std=c2x, there were three left-overs in musl that
> > still used that. Change them to use the correct prototype, since it is
> > available at all these places, anyhow.
> 
> I think there's one more:
> 
> diff --git a/src/process/posix_spawn.c b/src/process/posix_spawn.c
> index 8294598b..e05f1623 100644
> --- a/src/process/posix_spawn.c
> +++ b/src/process/posix_spawn.c
> @@ -149,7 +149,7 @@ static int child(void *args_vp)
>  		? &attr->__mask : &args->oldmask, 0);
>  
>  	int (*exec)(const char *, char *const *, char *const *) =
> -		attr->__fn ? (int (*)())attr->__fn : execve;
> +		attr->__fn ? (int (*)(const char *, char *const *, char *const *))attr->__fn : execve;
>  
>  	exec(args->path, args->argv, args->envp);
>  	ret = -errno;
> 
> Perhaps it's better to solve this by changing the type of __fn in
> posix_spawnattr_t from void * to the appropriate function type, but
> since this is in a public header, I'm not sure if it somehow breaks
> something:
> 
> diff --git a/include/spawn.h b/include/spawn.h
> index 8eb73e00..fec1280d 100644
> --- a/include/spawn.h
> +++ b/include/spawn.h
> @@ -29,7 +29,7 @@ typedef struct {
>  	pid_t __pgrp;
>  	sigset_t __def, __mask;
>  	int __prio, __pol;
> -	void *__fn;
> +	int (*__fn)(const char *, char *const *, char *const *);
>  	char __pad[64-sizeof(void *)];
>  } posix_spawnattr_t;
>  
> diff --git a/src/process/posix_spawn.c b/src/process/posix_spawn.c
> index 8294598b..2611fb94 100644
> --- a/src/process/posix_spawn.c
> +++ b/src/process/posix_spawn.c
> @@ -149,7 +149,7 @@ static int child(void *args_vp)
>  		? &attr->__mask : &args->oldmask, 0);
>  
>  	int (*exec)(const char *, char *const *, char *const *) =
> -		attr->__fn ? (int (*)())attr->__fn : execve;
> +		attr->__fn ? attr->__fn : execve;
>  
>  	exec(args->path, args->argv, args->envp);
>  	ret = -errno;
> diff --git a/src/process/posix_spawnp.c b/src/process/posix_spawnp.c
> index aad6133b..28ef1aa4 100644
> --- a/src/process/posix_spawnp.c
> +++ b/src/process/posix_spawnp.c
> @@ -8,6 +8,6 @@ int posix_spawnp(pid_t *restrict res, const char *restrict file,
>  {
>  	posix_spawnattr_t spawnp_attr = { 0 };
>  	if (attr) spawnp_attr = *attr;
> -	spawnp_attr.__fn = (void *)__execvpe;	
> +	spawnp_attr.__fn = __execvpe;	
>  	return posix_spawn(res, file, fa, &spawnp_attr, argv, envp);
>  }

It's intentional that public headers don't leak implementation
internals, so the void * with cast back to function pointer type is
preferred.

Rich