Ethereal-dev: Re: [Ethereal-dev] Van Jacobson dissector.
> You should stay away from using bitfields like this
> in portable code.
Given the fact that C compilers don't guarantee that the bit order
matches the byte order, etc.., one should probably stay away from using
bitfields, period, in data structures in network packets.
> We just had an issue with this type of struct-casting
> in tcp_graph.c. We stay away from this. Because hdr_buf is
> being allocated from a GMemChunk, it might be aligned
> properly for what you're doing.
...and because the first byte of the chunk is an offset, it's almost
certainly *not* aligned properly, as the IP header starts 1 byte off of
a 4-byte boundary. Works fine on an x86-based PC (unless some OS turns
the "require strict alignment" flag on 486 and later x86 processors);
works horribly, on, say, SPARC or Alpha.
The documentation for memory chunks in GLib 1.2:
http://developer.gnome.org/doc/API/glib/glib-memory-chunks.html
doesn't explicitly say that memory chunks are aligned on strict
boundaries; however, in order to be able to use pointers gotten from
"g_mem_chunk_alloc()" as pointers to structures, they would have to be
aligned on as strict a boundary as the machine and compiler require, so
they're presumably aligned.
However, as noted, the one-byte offset completely screws up the
alignment.
The fields being referred to are 1-byte fields or bitfields; however,
that's not sufficient to guarantee that the generated code won't do
unaligned references (consider, for example, an Alpha without BWX
instructions; I've seen GCC for Alpha generate code that assumes that
structure pointers are properly aligned and that crashes with alignment
faults if they're not properly aligned).
I'd be inclined to store 4 bytes of offset, rather than 1 byte, just to
align stuff properly. Given the fact that items allocated from memory
chunks have to be aligned neatly, the actual chunk of memory allocated
is probably padded out to 132 bytes (or 136 bytes) anyway....
> > /* Store the reconstructed header in frame data area */
> > buf_hdr = g_mem_chunk_alloc(vj_header_memchunk);
>
> I don't see a g_mem_chunk_free() call for the buf_hdr's that
> are allocated.
That's what the "g_mem_chunk_destroy()" call in "vj_init()" is for.
Those structures at attached to frames, so they have to be kept around
until a redissection is done or a new capture file is read.
