[Bug c/115185] Missing "too long" warning when string-array size doesn't include NULL byte

[alx at kernel dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115185

--- Comment #15 from Alejandro Colomar <alx at kernel dot org> ---
(In reply to Sergei Trofimovich from comment #14)
> This generates warning on reasonably looking code.
> 
> On strace-6.9:
> https://github.com/strace/strace/blob/v6.9/src/print_utils.h#L16
> 
>   In file included from v4l2.c:40:
>   print_utils.h:16:35: error: initializer-string for array of 'char' is too
> long [-Werror=unterminated-string-initialization]
>      16 | static const char hex_chars[16] = "0123456789abcdef";
>         |                                   ^~~~~~~~~~~~~~~~~~

This is the kind of code I'd warn about.  One may be tempted to do

    strchr(hex_chars, *p);

in a loop to parse a hex number from a string.  And then bug.
That would probably be triggered to easily, admittedly.

Surrounding that with a pragma to turn off the warning there
would let the programmer clearly know that it's not a string.

> 
> /cc Dmitry
> 
> On elfutils-0.191:
> https://sourceware.org/git/?p=elfutils.git;a=blob;f=backends/x86_64_regs.c;
> h=ef987daf43f01187c468d29e4d412ecafdcb2878;
> hb=18a015c0b0787ba5acb39801ab7c17dac50f584d#l85
> 
>   x86_64_regs.c: In function 'x86_64_register_info':
>   x86_64_regs.c:85:11: error: initializer-string for array of 'char' is too
> long [-Werror=unterminated-string-initialization]
>    85 |           "ax", "dx", "cx", "bx", "si", "di", "bp", "sp
>       |           ^~~~
> 
> /cc Mark
> 
> I would say both uses are reasonable even if they strip trailing '\0'.

Agree, which is why I didn't include it in Wall.

> 
> Warning-free code that does the equivalent might be a bit ugly:
> 
>     static const char hex_chars[17] = "0123456789abcdef"; // extra byte
> 
> or
> 
>     static const char hex_chars[16] = { '0', '1', ..., 'f' }; // lots of
> boilerplate

Agree, I don't like the workarounds.  Maybe a project that does that often
would better use -Wno-unterminated-string-initialization, or if it's only
sporadically, use a pragma around the specific uses.

> 
> WDYT of relaxing gcc warning a bit and not complain if the only thing it
> strips is a '\0'?

That's precisely the purpose of that warning: warn when it's an off-by-one,
so the '\0' is removed.

When it's an off-by-two-or-more, it's a (pedantic) error:


$ cat offbytwo.c 
char x[2] = "foo";


$ cc offbytwo.c -S
offbytwo.c:1:13: warning: initializer-string for array of ‘char’ is too long
    1 | char x[2] = "foo";
      |             ^~~~~


$ cc offbytwo.c -pedantic-errors -S
offbytwo.c:1:13: error: initializer-string for array of ‘char’ is too long
    1 | char x[2] = "foo";
      |             ^~~~~


> Otherwise at least strace and elfutils would have to adapt.

The only option I would see is to not enable this in -Wextra.  But I think the
implicit contract with -Wextra is that you want this kind of warnings that are
often useful but sometimes not.