[PATCH v3] tftp.h: rework layout to work with fortification

[Carlos O'Donell]

On Mon, May 7, 2012 at 7:04 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday 07 May 2012 11:52:18 Carlos O'Donell wrote:
>> On Mon, May 7, 2012 at 11:51 AM, Joseph S. Myers wrote:
>> > On Mon, 7 May 2012, Carlos O'Donell wrote:
>> >> On Mon, May 7, 2012 at 10:38 AM, Joseph S. Myers wrote:
>> >> > On Mon, 7 May 2012, Carlos O'Donell wrote:
>> >> >> It still doesn't work with -Wall -Werror -pedantic because zero sized
>> >> >> arrays are not allowed in ISO C.
>> >> >
>> >> > This should not give errors in system headers.  Have you found such an
>> >> > error with the header installed in a system header directory?  If so,
>> >> > that would be a GCC bug.
>> >>
>> >> No, I haven't tried installing this and testing.
>> >>
>> >> I didn't know that GCC had special handling for system headers.
>> >>
>> >> How does that work?
>> >
>> > Most warnings are not emitted for locations in system headers (those
>> > found in system header directories, with -isystem or marked with #pragma
>> > GCC system_header), unless -Wsystem-headers.  This applies before
>> > pedwarns are turned into errors.
>>
>> Thanks, I wasn't aware of this special handling.
>>
>> In that case if Mike can show that the installed header works with
>> -Werror -Wall -pedantic then I'm happy with the final version.
>
> $ echo '#include <arpa/tftp.h>' | gcc -x c -c - -Werror -Wall -pedantic
> $ echo $?
> 0

That's good enough for me.

> $ echo '#include <arpa/tftp.h>' | gcc -E -dD - | sed -n '/^struct tftphdr/,/^}/p'
> struct tftphdr {
>  short th_opcode;
>  union {
>  char tu_padding[3];
>  struct {
>   union {
>    unsigned short tu_block;
>    short tu_code;
>   } __attribute__ ((__packed__)) th_u3;
>   char tu_data[0];
>  } __attribute__ ((__packed__)) th_u2;
>  char tu_stuff[0];
>  } __attribute__ ((__packed__)) th_u1;
> } __attribute__ ((__packed__));
>
> i would have used flexible arrays but gcc doesn't allow those in unions:
> tftp.h:60:8: error: flexible array member in union
> -mike

I know, that would have been the best solution. You should file an
upstream enhancement request for this.

Cheers,
Carlos.