Subject: Re: bug#55895: [PATCH] maint: Fix ptr_align signature to silence -Wmaybe-uninitialized
From: Paul Eggert <[email-masked]>
To: Anders Kaseorg <[email-masked]>
Cc: [email-masked]
Date: Sat, 11 Jun 2022 09:12:08 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

On 6/10/22 21:11, Anders Kaseorg wrote:
> It seems the important step I should 
> have included was CFLAGS=-O0.

Ah, OK. Since you're building from Git, I can refer you to 
README-hacking which is intended for that. It says, "If you get warnings 
with other configurations, you can run
  './configure --disable-gcc-warnings' or 'make WERROR_CFLAGS='
  to build quietly or verbosely, respectively.
" Here, "other configurations" refers to what you're doing.

> (With GCC 12.1.1 I get the same error and also additional errors that might merit further investigation.) 

Like most static analysis tools, GCC generates a bunch of false 
positives unless you baby it just right. We do the babying only for the 
latest GCC with the default configuration; otherwise, it's typically not 
worth the trouble. Feel free to investigate the other warnings, but 
they're important only if they're true positives (and most likely 
they're not, because gcc -O0 is dumber than gcc -O2).

> there’s never a reason to call ptr_align with a const pointer, because if the memory is initialized the pointer would have already been aligned

First, a const pointer can point to uninitialized storage. Second, even 
if the referenced memory is initialized the pointer need not be aligned 
already. For example, this is valid:

     char *p = malloc (1024);
     if (!p) return;
     char const *q = p; // q points to uninitialized storage
     char const *r = ptr_align (q, 512); // q is not aligned already
     memset (p, 127, 1024);
     ...

Replacing 'malloc (1024)' with 'calloc (1024, 1)' (thus initializing the 
storage before aligning the pointer) wouldn't affect the validity of the 
code.

> Also, the current signature converts a const pointer to a mutable pointer.

Yes, it's like strchr which is annoying but that's the best C can do.

You're right that changing it from void const * to void * won't hurt 
coreutils' current callers but I'd rather not massage the code merely to 
pacify nondefault configurations. There are too many nondefault 
configurations to worry about and massaging the code to pacify them all 
would waste our time and confuse the code. Instead, we pacify only 
default configurations with current GCC.