On Mon, Oct 4, 2021 at 12:55 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > > Hi Arnd, > > On 27/09/2021 14:20, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@xxxxxxxx> > > > > 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
