linux-kernel - Re: [PATCH] media: gspca/gl860-mi1320: avoid -Wstring-concatenation warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAK8P3a0ZOO-1QHAQbhPqFe9URX77bXORCybemLyd8DNQC-wVQw@mail.gmail.com>
Date:   Mon, 4 Oct 2021 13:38:23 +0200
From:   Arnd Bergmann <arnd@...nel.org>
To:     Hans Verkuil <hverkuil@...all.nl>
Cc:     Olivier Lorin <o.lorin@...oste.net>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Arnd Bergmann <arnd@...db.de>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        llvm@...ts.linux.dev
Subject: Re: [PATCH] media: gspca/gl860-mi1320: avoid -Wstring-concatenation warning

On Mon, Oct 4, 2021 at 12:55 PM Hans Verkuil <hverkuil@...all.nl> wrote:
>
> Hi Arnd,
>
> On 27/09/2021 14:20, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@...db.de>
> >
> > Newer clang versions are suspicious of definitions that mix concatenated
> > strings with comma-separated arrays of strings, this has found real bugs
> > elsewhere, but this seems to be a false positive:
> >
> > drivers/media/usb/gspca/gl860/gl860-mi1320.c:62:37: error: suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma? [-Werror,-Wstring-concatenation]
> >         "\xd3\x02\xd4\x28\xd5\x01\xd0\x02" "\xd1\x18\xd2\xc1"
> >                                            ^
> >                                           ,
> > drivers/media/usb/gspca/gl860/gl860-mi1320.c:62:2: note: place parentheses around the string literal to silence warning
> >         "\xd3\x02\xd4\x28\xd5\x01\xd0\x02" "\xd1\x18\xd2\xc1"
> >
> > Use the extra parentheses as suggested in the warning message.
>
> I noticed that gl860-ov9655.c uses the same construct, doesn't that produce the
> same warning?

Curiously, it does not. I tried reproducing this in godbolt.org,
see https://godbolt.org/z/W6K69qcz3

For some reason, clang only warns about some of those, and it appears to
depend on the ratio between string concatenations and array elements.

> Also, does clang only warn about 'static u8 *tbl[]' initializers, or also
> for 'static u8 *tbl' initializers (i.e. not a pointer array) with the same
> string concatenation?

When there is only one element rather than an array, it does not warn,
because it's not a mix of concatenation and array elements.

> I made a patch that replaces these ugly hex strings with compound initializers:
>
> static u8 *tbl_640[] = {
>         (u8[]){
>                 0x0d, 0x80, 0xf1, 0x08, 0x03, 0x04, 0xf1, 0x04,
>                 0x04, 0x05, 0xf1, 0x02, 0x07, 0x01, 0xf1, 0x7c,
>                 0x08, 0x00, 0xf1, 0x0e, 0x21, 0x80, 0xf1, 0x00,
>                 0x0d, 0x00, 0xf1, 0x08, 0xf0, 0x00, 0xf1, 0x01,
>                 0x34, 0x10, 0xf1, 0x10, 0x3a, 0x43, 0xf1, 0x00,
>                 0xa6, 0x05, 0xf1, 0x02, 0xa9, 0x04, 0xf1, 0x04,
>                 0xa7, 0x02, 0xf1, 0x81, 0xaa, 0x01, 0xf1, 0xe2,
>                 0xae, 0x0c, 0xf1, 0x09
>         }, (u8[]){
>                 0xf0, 0x00, 0xf1, 0x02, 0x39, 0x03, 0xf1, 0xfc,
>                 0x3b, 0x04, 0xf1, 0x04, 0x57, 0x01, 0xf1, 0xb6,
>                 0x58, 0x02, 0xf1, 0x0d, 0x5c, 0x1f, 0xf1, 0x19,
>                 0x5d, 0x24, 0xf1, 0x1e, 0x64, 0x5e, 0xf1, 0x1c,
>                 0xd2, 0x00, 0xf1, 0x00, 0xcb, 0x00, 0xf1, 0x01
>         }, (u8[]){
>                 0xd3, 0x02, 0xd4, 0x10, 0xd5, 0x81, 0xd0, 0x02,
>                 0xd1, 0x08, 0xd2, 0xe1
>         }
> };
>
> but it clang also warns about 'static u8 *tbl' multi-string initializers,
> then it would make sense to replace all these hex-strings. It's rather
> ugly.

This seems fine.

       Arnd