diff mbox series

lib/libnet/ipv6: Silence compiler warning from Clang

Message ID 20220627082632.25192-1-thuth@redhat.com
State New
Headers show
Series lib/libnet/ipv6: Silence compiler warning from Clang | expand

Commit Message

Thomas Huth June 27, 2022, 8:26 a.m. UTC
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(-)

Comments

Segher Boessenkool June 27, 2022, 10:05 p.m. UTC | #1
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
Alexey Kardashevskiy June 28, 2022, 3:35 a.m. UTC | #2
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);
Thomas Huth June 28, 2022, 6:34 a.m. UTC | #3
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
Alexey Kardashevskiy June 28, 2022, 8:40 a.m. UTC | #4
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.
Segher Boessenkool June 28, 2022, 3:19 p.m. UTC | #5
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
Thomas Huth June 30, 2022, 5:02 p.m. UTC | #6
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
Segher Boessenkool June 30, 2022, 5:17 p.m. UTC | #7
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 mbox series

Patch

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);