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

[Thomas Huth]

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?

> +               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?

 Thomas