[SLOF] [PATCH] Fix remaining compiler warnings in sloffs.c

[Nikunj A Dadhania]

Thomas Huth <thuth at redhat.com> writes:

> On 09.08.2016 04:16, Nikunj A Dadhania wrote:
>> Segher Boessenkool <segher at kernel.crashing.org> writes:
>> 
>>> On Mon, Aug 08, 2016 at 02:45:35PM +0530, Nikunj A Dadhania wrote:
>>>>>>> In the end, we just might want to add -fno-strict-aliasing to the
>>>>>>> HOSTCFLAGS, just like we already did it in make.rules for the normal
>>>>>>> CFLAGS, and call it a day.
>>>>>>
>>>>>> Or you could fix the problems.  SLOF used to work with -fstrict-aliasing,
>>>>>> and it was a nice performance win.
>>>>>
>>>>> That's a question for Nikunj who committed that change a couple of years
>>>>> ago ...
>>>>> (see http://git.qemu.org/?p=SLOF.git;a=commitdiff;h=7eca6a5e56f468)
>>>>
>>>> I remember that we had a discussion on this and BenH had suggested that
>>>> we cannot re-audit all the code for correctness with strict-aliasing. So
>>>> disable it.
>>>
>>> That's what -Wstrict-aliasing is for (needs -fstrict-aliasing active as
>>> well, to work).
>>>
>>> Rewriting the code to make it better can be quite a bit of work, of
>>> course.  But you could e.g. only use -fno-strict-aliasing on those files
>>> where -Wstrict-aliasing warns.
>> 
>> I think this is what is needed. I have to test this well, as I am not
>> exactly sure what was the way we were hitting this.
>> 
>> diff --git a/clients/net-snk/app/netlib/ipv4.c b/clients/net-snk/app/netlib/ipv4.c
>> index 3a1a789..157d71c 100644
>> --- a/clients/net-snk/app/netlib/ipv4.c
>> +++ b/clients/net-snk/app/netlib/ipv4.c
>> @@ -547,7 +547,7 @@ static void fill_udp_checksum(struct iphdr *ipv4_hdr)
>>         unsigned i;
>>         unsigned long checksum = 0;
>>         struct iphdr ip_hdr;
>> -       char *ptr;
>> +       uint16_t *ptr;
>>         udp_hdr_t *udp_hdr;
>>  
>>         udp_hdr = (udp_hdr_t *) (ipv4_hdr + 1);
>> @@ -559,13 +559,13 @@ static void fill_udp_checksum(struct iphdr *ipv4_hdr)
>>         ip_hdr.ip_len    = udp_hdr->uh_ulen;
>>         ip_hdr.ip_p      = ipv4_hdr->ip_p;
>>  
>> -       ptr = (char*) udp_hdr;
>> -       for (i = 0; i < udp_hdr->uh_ulen; i+=2)
>> -               checksum += *((uint16_t*) &ptr[i]);
>> +       ptr = (uint16_t*) udp_hdr;
>> +       for (i = 0; i < udp_hdr->uh_ulen; i++)
>
> Don't you need to rather check for " i < udp_hdr->uh_ulen / 2" now?

Right, missed that. And I think we need to take care for odd length as
well.

i < (udp_hdr->uh_ulen + 1)/ 2

>> +               checksum += ptr[i];
>>  
>> -       ptr = (char*) &ip_hdr;
>> -       for (i = 0; i < sizeof(struct iphdr); i+=2)
>> -               checksum += *((uint16_t*) &ptr[i]);
>> +       ptr = (uint16_t*) &ip_hdr;
>> +       for (i = 0; i < sizeof(struct iphdr); i++)
>> +               checksum += ptr[i];
>
> Dito, the limit should be divided by 2 now?

Sure.

Regards,
Nikunj