| Message ID | 20220627082632.25192-1-thuth@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | lib/libnet/ipv6: Silence compiler warning from Clang | expand |
On Mon, Jun 27, 2022 at 10:26:32AM +0200, Thomas Huth wrote: > When compiling the libnet code with Clang (e.g. for the s390-ccw bios), > it complains with the following warning: > > ipv6.c:447:18: warning: variable length array folded to constant array > as an extension [-Wgnu-folding-constant] > unsigned short raw[ip6size]; > ^ > The warning is completely harmless, of course. Anyway let's rewrite the > code a little bit to make the compiler silent again. This makes the code worse though :-( You could shut off the silly warning instead? Clang claims to be compatible to GCC, and GCC explicitly allows variable-length auto arrays even in C90 mode. This is documented, too. [ That it is "folded to a constant array" is a) completely untrue, and b) that it is folded to a constant _size_ array is just a trivial and obvious optimisation, that you can expect any good compiler to do. ] Segher
On 6/27/22 18:26, Thomas Huth wrote: > When compiling the libnet code with Clang (e.g. for the s390-ccw bios), > it complains with the following warning: > > ipv6.c:447:18: warning: variable length array folded to constant array > as an extension [-Wgnu-folding-constant] > unsigned short raw[ip6size]; > ^ > The warning is completely harmless, of course. Anyway let's rewrite the > code a little bit to make the compiler silent again. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > lib/libnet/ipv6.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/lib/libnet/ipv6.c b/lib/libnet/ipv6.c > index 6420004..259087b 100644 > --- a/lib/libnet/ipv6.c > +++ b/lib/libnet/ipv6.c > @@ -441,10 +441,9 @@ static unsigned short ip6_checksum(struct ip6hdr *ip6h, unsigned char *packet, > { > int i; > unsigned long checksum; > - const int ip6size = sizeof(struct ip6hdr)/sizeof(unsigned short); > union { > struct ip6hdr ip6h; > - unsigned short raw[ip6size]; > + uint16_t raw[sizeof(struct ip6hdr) / sizeof(uint16_t)]; > } pseudo; > > memcpy (&pseudo.ip6h, ip6h, sizeof(struct ip6hdr)); > @@ -455,7 +454,7 @@ static unsigned short ip6_checksum(struct ip6hdr *ip6h, unsigned char *packet, > for (checksum = 0, i = 0; i < bytes; i += 2) > checksum += (packet[i] << 8) | packet[i + 1]; > > - for (i = 0; i < ip6size; i++) > + for (i = 0; i < (int)(sizeof(pseudo.raw) / sizeof(pseudo.raw[0])); i++) ARRAY_SIZE()? > checksum += pseudo.raw[i]; > > checksum = (checksum >> 16) + (checksum & 0xffff);
On 28/06/2022 00.05, Segher Boessenkool wrote: > On Mon, Jun 27, 2022 at 10:26:32AM +0200, Thomas Huth wrote: >> When compiling the libnet code with Clang (e.g. for the s390-ccw bios), >> it complains with the following warning: >> >> ipv6.c:447:18: warning: variable length array folded to constant array >> as an extension [-Wgnu-folding-constant] >> unsigned short raw[ip6size]; >> ^ >> The warning is completely harmless, of course. Anyway let's rewrite the >> code a little bit to make the compiler silent again. > > This makes the code worse though :-( > > You could shut off the silly warning instead? Clang claims to be > compatible to GCC, and GCC explicitly allows variable-length auto > arrays even in C90 mode. This is documented, too. Ok, can do, SLOF itself cannot be compiled with Clang anyway, so it likely makes more sense indeed to disable this warning in the s390-ccw bios that uses SLOF's libnet. Thomas
On 6/28/22 16:34, Thomas Huth wrote: > On 28/06/2022 00.05, Segher Boessenkool wrote: >> On Mon, Jun 27, 2022 at 10:26:32AM +0200, Thomas Huth wrote: >>> When compiling the libnet code with Clang (e.g. for the s390-ccw bios), >>> it complains with the following warning: >>> >>> ipv6.c:447:18: warning: variable length array folded to constant array >>> as an extension [-Wgnu-folding-constant] >>> unsigned short raw[ip6size]; >>> ^ >>> The warning is completely harmless, of course. Anyway let's rewrite the >>> code a little bit to make the compiler silent again. >> >> This makes the code worse though :-( >> >> You could shut off the silly warning instead? Clang claims to be >> compatible to GCC, and GCC explicitly allows variable-length auto >> arrays even in C90 mode. This is documented, too. > > Ok, can do, SLOF itself cannot be compiled with Clang anyway, so it > likely makes more sense indeed to disable this warning in the s390-ccw > bios that uses SLOF's libnet. I rather like the initial idea of getting rid of ip6size, I tend to (incorrectly) read "ip6size" as "size of the ipv6 header in bytes" (as sizes are almost always bytes) which it is not here.
Hi! On Tue, Jun 28, 2022 at 06:40:23PM +1000, Alexey Kardashevskiy wrote: > On 6/28/22 16:34, Thomas Huth wrote: > >On 28/06/2022 00.05, Segher Boessenkool wrote: > >>On Mon, Jun 27, 2022 at 10:26:32AM +0200, Thomas Huth wrote: > >>>When compiling the libnet code with Clang (e.g. for the s390-ccw bios), > >>>it complains with the following warning: > >>> > >>> ipv6.c:447:18: warning: variable length array folded to constant array > >>> as an extension [-Wgnu-folding-constant] > >>> unsigned short raw[ip6size]; > >>> ^ > >>>The warning is completely harmless, of course. Anyway let's rewrite the > >>>code a little bit to make the compiler silent again. > >> > >>This makes the code worse though :-( > >> > >>You could shut off the silly warning instead? Clang claims to be > >>compatible to GCC, and GCC explicitly allows variable-length auto > >>arrays even in C90 mode. This is documented, too. > > > >Ok, can do, SLOF itself cannot be compiled with Clang anyway, so it > >likely makes more sense indeed to disable this warning in the s390-ccw > >bios that uses SLOF's libnet. > > I rather like the initial idea of getting rid of ip6size, I tend to > (incorrectly) read "ip6size" as "size of the ipv6 header in bytes" (as > sizes are almost always bytes) which it is not here. It could use a better name no matter what, it doesn't even say "header" :-) Using ARRAY_SIZE this will probably look a lot better, with or without the change? Segher
On 28/06/2022 17.19, Segher Boessenkool wrote: > Hi! > > On Tue, Jun 28, 2022 at 06:40:23PM +1000, Alexey Kardashevskiy wrote: >> On 6/28/22 16:34, Thomas Huth wrote: >>> On 28/06/2022 00.05, Segher Boessenkool wrote: >>>> On Mon, Jun 27, 2022 at 10:26:32AM +0200, Thomas Huth wrote: >>>>> When compiling the libnet code with Clang (e.g. for the s390-ccw bios), >>>>> it complains with the following warning: >>>>> >>>>> ipv6.c:447:18: warning: variable length array folded to constant array >>>>> as an extension [-Wgnu-folding-constant] >>>>> unsigned short raw[ip6size]; >>>>> ^ >>>>> The warning is completely harmless, of course. Anyway let's rewrite the >>>>> code a little bit to make the compiler silent again. >>>> >>>> This makes the code worse though :-( >>>> >>>> You could shut off the silly warning instead? Clang claims to be >>>> compatible to GCC, and GCC explicitly allows variable-length auto >>>> arrays even in C90 mode. This is documented, too. >>> >>> Ok, can do, SLOF itself cannot be compiled with Clang anyway, so it >>> likely makes more sense indeed to disable this warning in the s390-ccw >>> bios that uses SLOF's libnet. >> >> I rather like the initial idea of getting rid of ip6size, I tend to >> (incorrectly) read "ip6size" as "size of the ipv6 header in bytes" (as >> sizes are almost always bytes) which it is not here. > > It could use a better name no matter what, it doesn't even say "header" :-) > > Using ARRAY_SIZE this will probably look a lot better, with or without > the change? FYI, I'm going with this patch for the s390-ccw bios: https://lists.gnu.org/archive/html/qemu-devel/2022-06/msg05306.html I think that's the right way to go since SLOF cannot be compiled with Clang yet. Thomas
On Thu, Jun 30, 2022 at 07:02:48PM +0200, Thomas Huth wrote: > FYI, I'm going with this patch for the s390-ccw bios: > > https://lists.gnu.org/archive/html/qemu-devel/2022-06/msg05306.html > > I think that's the right way to go since SLOF cannot be compiled with Clang > yet. Looks good! +1 Segher
diff --git a/lib/libnet/ipv6.c b/lib/libnet/ipv6.c index 6420004..259087b 100644 --- a/lib/libnet/ipv6.c +++ b/lib/libnet/ipv6.c @@ -441,10 +441,9 @@ static unsigned short ip6_checksum(struct ip6hdr *ip6h, unsigned char *packet, { int i; unsigned long checksum; - const int ip6size = sizeof(struct ip6hdr)/sizeof(unsigned short); union { struct ip6hdr ip6h; - unsigned short raw[ip6size]; + uint16_t raw[sizeof(struct ip6hdr) / sizeof(uint16_t)]; } pseudo; memcpy (&pseudo.ip6h, ip6h, sizeof(struct ip6hdr)); @@ -455,7 +454,7 @@ static unsigned short ip6_checksum(struct ip6hdr *ip6h, unsigned char *packet, for (checksum = 0, i = 0; i < bytes; i += 2) checksum += (packet[i] << 8) | packet[i + 1]; - for (i = 0; i < ip6size; i++) + for (i = 0; i < (int)(sizeof(pseudo.raw) / sizeof(pseudo.raw[0])); i++) checksum += pseudo.raw[i]; checksum = (checksum >> 16) + (checksum & 0xffff);
When compiling the libnet code with Clang (e.g. for the s390-ccw bios), it complains with the following warning: ipv6.c:447:18: warning: variable length array folded to constant array as an extension [-Wgnu-folding-constant] unsigned short raw[ip6size]; ^ The warning is completely harmless, of course. Anyway let's rewrite the code a little bit to make the compiler silent again. Signed-off-by: Thomas Huth <thuth@redhat.com> --- lib/libnet/ipv6.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)