[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 16 Apr 2024 09:42:57 -0400
From: Rich Felker <dalias@...c.org>
To: Michael Forney <mforney@...rney.org>
Cc: Jens Gustedt <Jens.Gustedt@...ia.fr>, musl@...ts.openwall.com
Subject: Re: Re: [PATCH] C23: update some legacy function pointers
On Mon, Apr 15, 2024 at 08:57:35PM -0700, Michael Forney wrote:
> Jens Gustedt <Jens.Gustedt@...ia.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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
