On Wed, Aug 14, 2002 at 04:44:02PM +0200, Yaniv Kaul wrote:
> I've noticed packet-isakmp.c does:
> hdr = (struct isakmp_hdr *)tvb_get_ptr(tvb, 0, sizeof (struct isakmp_hdr));
>
> which according to README.developer is no-no-no ("Don't fetch data from
> packets by getting a pointer to data in the packet
> with "tvb_get_ptr()", casting that pointer to a pointer to a structure,
> and dereferencing that pointer.").
>
> I'll fix it if I can get an authoritive 'yes, it's wrong' answr.
The reason why README.developer says "don't do that" is, to quote my
reply to your question about the various ntoh* routines:
ntohl():
...
is passed a 32-bit integral value, but there's no
guarantee that a structure pointer set from
"tvb_get_ptr()" is properly aligned, so if you do
structp = tvb_get_ptr(tvb, offset, sizeof (struct));
foo = ntohl(structp->field);
you run the risk of getting an alignment fault on many
platforms (including the one I use at work, namely
SPARC/Solaris)
Another reason to avoid it is also given in that message:
That's the best one to use. If you use "tvb_get_ptr()" and then
extract a bunch of stuff by hand, that throws an exception
unless the *entire* structure on which you're doing the
"tvb_get_ptr()" is available in the frame, but if you get each
individual field with, for example, "tvb_get_ntohl()", the
exception won't be thrown until you fetch a field that's not
available.
That means that if you have, for example, a short frame
(captured with a snapshot length less than the length of the
frame), you will get more stuff in the protocol tree.
If you don't care about short frames, and if you don't refer to any
structure members with alignment requirements (except by passing
pointers to them to "pntohl()" or other routines/macros that don't
assume the pointer to be aligned), you can get away with it.
(There are also the risks that
1) the structure members might not be aligned the way they are
in the packet, due to those alignment requirements - and
using "#pragma pack" and the like is *not* a valid fix, as
those are compiler-dependent, and Ethereal has to be
compilable with MSVC++, GCC, *and* various vendor compilers)
and
2) the structure *size* might not be what you'd expect - I have
the impression that at least some versions of GCC for ARM,
for example, pad structures to a multiple of 4 bytes *even if
no structure member requires 4-byte alignment*.)