On Tue, Nov 16, 2010 at 01:04:48PM +0300, Vadim Zhukov wrote:
> 2010/11/16 Vadim Zhukov <persg...@gmail.com>:
> > 2010/11/16 Stuart Henderson <s...@spacehopper.org>:
> >> On 2010/11/16 04:50, Vadim Zhukov wrote:
> >> [..]
> >>
> >> Great stuff, this has annoyed me for a while :) I can't test today but
> >> will try and do that soon. I won't be able to really test pfsync though..
> >>
> >>> B - More style(9) compliance. Not sure about struct pf_state change,
> >>> B B though: will it ever break on architectures with 4-byte pointers
> >>> B B but 64-bit alignment requirements, if there are any?.. Sorry for
> >>> B B possibly stupid question.
> >>
> >> Afaik the strictest requirements on any arch we support is that
> >> a data type must be aligned to the size of that data type. 32-bit
> >> aligned for 4-byte types, 64-bit aligned for 8-byte types, etc.
>
> Then the snippet should become:
>
> @@ -807,6 +808,8 @@ struct pf_state {
> struct pf_state_key *key[2]; /* addresses stack and wire
> */
> struct pfi_kif *kif;
> struct pfi_kif *rt_kif;
> + struct pf_rule *rt_rule;
> + caddr_t align;
> u_int64_t packets[2];
> u_int64_t bytes[2];
> u_int32_t creation;
>
> Am I right?
I don't think so. It's the compiler's job to align the struct fields
properly.
As long as you do not access the field using pointer tricks you're
safe. I do not know if pf is doing that. Another reason to do "manual"
layout is when the struct is used on the wire.
There is a guideline to lay out structs starting with the biggest
field and then working to smaller ones. This guideline is ambiguous
since the size of fields is depedent on the target. Still, some effort
wrt this is preferred since it makes sure the compiler has to insert
padding only in rare circumstances.
-Otto
