Re: [lwip-users] [PATCH 6/7] lwip: fix warning: 'packed' attributeignored
On 9/21/07, Kieran Mansley <[EMAIL PROTECTED]> wrote: > > I do find it a little odd that the compiler thinks these types do not > need packing though. Although gcc might align things to the natural > size of the type (and so u8_t will always be "packed") I don't think > it's a definite requirement that a compiler do this. Keiran, My understanding of gcc is that u8_t is naturally packed, unless you change it's alignment with __attribute__((align)) The definition of the struct needs packing specified if that is what you want. Each instance of that struct will then be consistent. In reply to your other email, I don't really like the NOPACK_* name either, but other options I came up with seemed to get rather long. I can regenerate the patch with an alternate name if there is agreement. I'm happy to post patches inline, as attachments or just put them in the tracker. Most projects I've been involved with prefer inline so that's my default behaviour. I didn't put them in the patch tracker as they seemed to fit the "Trivial patches" category in this section of the wiki http://lwip.scribblewiki.com/Contributing_to_lwIP. All comments appreciated, this is healthy "peer review" and not taken as criticism. Andrew ___ lwip-users mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/lwip-users
RE: [lwip-users] [PATCH 6/7] lwip: fix warning: 'packed' attributeignored
On Fri, 2007-09-21 at 14:09 +0200, Goldschmidt Simon wrote: > > > - PACK_STRUCT_FIELD(struct ip_addr src); > > > - PACK_STRUCT_FIELD(struct ip_addr dest); > > > + NOPACK_STRUCT_FIELD(struct ip_addr src); > > > + NOPACK_STRUCT_FIELD(struct ip_addr dest); > > > > I'm happy with this in principle - avoiding compiler > > warnings, if they're genuine, is on the whole a good idea - > > but I don't like the "NOPACK" name. How about calling it > > PACK_STRUCT_STRUCTFIELD instead? > > Not the most elegant of names perhaps, but better defines > > what it's trying to achieve. > > And what about the u8_t types, then? If they also don't require packing, > we would have to define something like PACK_STRUCT_BYTE_FIELD... Yes. I'd also be happy with just not wrapping those entries with PACK_STRUCT_FIELD if others would prefer that. To be honest, I'm happy with it as it is if that's the consensus! While it's good to avoid the warning, it's not the most important thing. I do find it a little odd that the compiler thinks these types do not need packing though. Although gcc might align things to the natural size of the type (and so u8_t will always be "packed") I don't think it's a definite requirement that a compiler do this. I seem to remember this being discussed quite heatedly at length a few years ago though, so I'm keen to avoid that again. Kieran ___ lwip-users mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/lwip-users
RE: [lwip-users] [PATCH 6/7] lwip: fix warning: 'packed' attributeignored
> > - PACK_STRUCT_FIELD(struct ip_addr src); > > - PACK_STRUCT_FIELD(struct ip_addr dest); > > + NOPACK_STRUCT_FIELD(struct ip_addr src); > > + NOPACK_STRUCT_FIELD(struct ip_addr dest); > > I'm happy with this in principle - avoiding compiler > warnings, if they're genuine, is on the whole a good idea - > but I don't like the "NOPACK" name. How about calling it > PACK_STRUCT_STRUCTFIELD instead? > Not the most elegant of names perhaps, but better defines > what it's trying to achieve. And what about the u8_t types, then? If they also don't require packing, we would have to define something like PACK_STRUCT_BYTE_FIELD... Simon ___ lwip-users mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/lwip-users
