On Mon, Feb 12, 2018 at 6:11 PM, Willy Tarreau <w...@1wt.eu> wrote:
> On Mon, Feb 12, 2018 at 01:53:57PM +0100, Miguel Ojeda wrote:
>> > diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
>> > index ea7869c..d288900 100644
>> > --- a/drivers/auxdisplay/panel.c
>> > +++ b/drivers/auxdisplay/panel.c
>> > @@ -1506,10 +1506,10 @@ static struct logical_input *panel_bind_key(const
>> > char *name, const char *press,
>> > key->rise_time = 1;
>> > key->fall_time = 1;
>> > - strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str));
>> > - strncpy(key->u.kbd.repeat_str, repeat,
>> > sizeof(key->u.kbd.repeat_str));
>> > + strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str)
>> > - 1);
>> > + strncpy(key->u.kbd.repeat_str, repeat,
>> > sizeof(key->u.kbd.repeat_str) - 1);
>> > strncpy(key->u.kbd.release_str, release,
>> > - sizeof(key->u.kbd.release_str));
>> > + sizeof(key->u.kbd.release_str) - 1);
>> Are you sure about this patch? `kbd` says "strings can be non
>> Willy, maybe those should just be memcpy()s? (unless the remaining
>> bytes, if any, must be 0).
> For me this seems to be the result of yet another very stupid gcc warning
> trying to dissuade us from using well defined fonctions... it's unimaginable
> how gcc warnings have become stupid and irrelevant since its developers
> stopped using C to write it :-(
> If you want to work around this wrong warning, probably that increasing the
> destination storage size by one and adding -1 to strncpy() would shut it up
It does indeed.
> but that really becomes quite annoying to have to modify code and storage
> just to shut down a dumbass compiler trying to be smart.
I have looked a bit deeper at this new warning. It comes with -Wall,
and to trigger it the compiler needs to have some optimization passes
enabled (-O2 works). See https://godbolt.org/g/dydPah to play with it
Assuming gcc 8 comes out with this warning implemented as of today
(likely), we have several options then:
a) -Wno-stringop-truncation for gcc >= 8. Note: the warning checks
for several kinds of potential issues, some might be useful.
b) Use the new __attribute__((nonstring)) for gcc >= 8. For
instance, the kbd strings are anyhow marked with the /* strings can be
non null-terminated */ comment, so we might just as well replace the
comment with the attribute.
c) For this case: +1 the array length, -1 the sizeof; as Willy
suggested; or similar workarounds.
d) For this case: try to make gcc see that the source is a small
enough array. Unlikely, due to the keypad_profile indirection and the
loop in keypad_init.
Since anyway we will be hit by this warning in N places in the kernel
now or in the future, let's ask Linus et. al. about what should be the
policy to follow :)