Re: [lwip-users] Compiler warnings

[Jonathan Larmour]

Goldschmidt Simon wrote:
> Christiaan (and all others interested in getting clean code :),
>  
> First, I would like to thank you for the fast integration of parts of
> the patch, really nice work. I upgraded to the new version but still got
> a few warnings. Apart from a signed/unsigned issue in socket.c
> (resulting from sizeof() used in macro on line 90/91 being unsigned
> while 'err' in line 94 is of type err_t which is signed), those are
> mainly memory-alignment warnings (e.g. casts from u8_t to struct mem* in
> mem.c, also in memp.c and pbuf.c).
>
> Have you been able to compile the stack cleanly? If so, what platform
> are you using? On a platform like x86 or an 8-bit-platform, those
> warnings don't come, of course. Even on our NIOS-II platform, GCC only
> generates this warning if the -Wcast-align switch is turned on.
> Nevertheless, I think this is a severe warning which should be taken
> care of!
>
> The third issue is that GCC warns me about indexing an array with a
> 'char' in etharp.c (lines 369, 449 & 751). Don't know why this doesn't
> work. 'Signed char' and 'int' work, though...

It's simply following what the C standard says: "One of the expressions 
shall have type ‘‘pointer to object type’’, the other expression shall
have integer type, and the result has type ‘‘type’’."

A simple cast to int (e.g. arp_table[(int)i].state or whatever) may be 
sufficient. I've no idea why signed char works though!

> Fourth and last problem is that 'struct sockaddr' and 'struct
> sockaddr_in' don't have the same alignment since 'struct sockaddr' only
> has u8_t/char members. This can me fixed by declaration 'struct
> sockaddr_in' as packed, although I don't see the need for this.

Anonymous unions would be nice, but that's too much of an extension :-). 
One solution might be a macro (to be defined in cc.h) to allow alignment, 
so in gcc that would be __attribute__((aligned(something))). But I think 
an appropriate solution would be to add an element with stronger 
alignment, e.g.

struct sockaddr {
  u8_t sa_len;
  u8_t sa_family;
  u16_t sa_dummy;
  char sa_data[12];
};

According to the C standard, that may not strictly be sufficient. But I 
think it might work everywhere in practice.

> And for the mem-alignment bugs in mem.c, memp.c and pbuf.c: I don't
> think it's the best approach to use u8-arrays for the pools. Better
> would be to use arrays of the structs the pools have to use. This way,
> the mem-alignment would be taken care of by the compiler (generating the
> arrays) instead of the programmer.

It almost certainly wouldn't save any space though, just make it more 
obvious what's going on.

Jifl
--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
------["The best things in life aren't things."]------      Opinions==mine