Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230529154909.GU4163@brightrain.aerifal.cx>
Date: Mon, 29 May 2023 11:49:10 -0400
From: Rich Felker <dalias@...c.org>
To: Élie Brami <elie.brami@...ta.fr>
Cc: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: Re: [PATCH] Update __procfdname and seed48 proto to conform
 to header

On Mon, May 29, 2023 at 12:18:47AM +0000, Élie Brami wrote:
> In new GCC11 when -Werror=array-parameter=1 is added we get some mismatch
> 
> src/internal/procfdname.c:3:25: error: argument 1 of type 'char *' declared as a pointer [-Werror=array-parameter=]
>     3 | void __procfdname(char *buf, unsigned fd)
>       |                   ~~~~~~^~~
> In file included from src/internal/procfdname.c:1:
> src/internal/syscall.h:394:31: note: previously declared as an array 'char[static 27]'
>   394 | hidden void __procfdname(char __buf[static 15+3*sizeof(int)], unsigned);
>       |                          ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
> make: *** [Makefile:150: obj/src/internal/procfdname.o] Error 1
> src/prng/seed48.c:5:40: error: argument 1 of type 'short unsigned int *' declared as a pointer [-Werror=array-parameter=]
>     5 | unsigned short *seed48(unsigned short *s)
>       |                        ~~~~~~~~~~~~~~~~^
> In file included from ./src/include/stdlib.h:4,
>                  from src/prng/seed48.c:1:
> ../src/include/../../include/stdlib.h:135:25: note: previously declared as an array 'short unsigned int[3]'
>   135 | unsigned short *seed48 (unsigned short [3]);
>       |                         ^~~~~~~~~~~~~~~~~~
> cc1: some warnings being treated as errors
> make: *** [Makefile:150: obj/src/prng/seed48.o] Error 1
> ---
>  src/internal/procfdname.c | 2 +-
>  src/prng/seed48.c         | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/internal/procfdname.c b/src/internal/procfdname.c
> index fd7306ab..bfa3e7e5 100644
> --- a/src/internal/procfdname.c
> +++ b/src/internal/procfdname.c
> @@ -1,6 +1,6 @@
>  #include "syscall.h"
>  
> -void __procfdname(char *buf, unsigned fd)
> +void __procfdname(char buf[static 15+3*sizeof(int)], unsigned fd)
>  {
>          unsigned i, j;
>          for (i=0; (buf[i] = "/proc/self/fd/"[i]); i++);
> diff --git a/src/prng/seed48.c b/src/prng/seed48.c
> index bce7b339..6219ebcf 100644
> --- a/src/prng/seed48.c
> +++ b/src/prng/seed48.c
> @@ -2,7 +2,7 @@
>  #include <string.h>
>  #include "rand48.h"
>  
> -unsigned short *seed48(unsigned short *s)
> +unsigned short *seed48(unsigned short s[static 3])
>  {
>          static unsigned short p[3];
>          memcpy(p, __seed48, sizeof p);
> -- 
> 2.40.1

Is this a constraint violation we have? If so, then by all means it
should be changed. It might also should be changed if it's misleading
or not doing what we intend for it to be doing, but "gcc has a warning
that says this is wrong" is not a valid motivation for a change.

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.