Ethereal-dev: Re: [Ethereal-dev] packet-isakmp.c using tvb_get_ptr() and casts pointer to stru

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: "Ronnie Sahlberg" <sahlberg@xxxxxxxxxxxxxxxx>
Date: Thu, 15 Aug 2002 15:40:46 +1000
----- Original Message -----
From: "Yaniv Kaul"
Sent: Thursday, August 15, 2002 12:44 AM
Subject: [Ethereal-dev] packet-isakmp.c using tvb_get_ptr() and casts
pointer to struct...bad?


> 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.
>
> TIA,
> Y.

Yes it is wrong and will break on many platforms. Please fix it it.

First it does not guarantee alignment of the fields any more something which
may dump core
on some platyforms which does not like unaligned accesses.

Second, it will only work for platforms where ntohl is a NOP. I.e. only on
platforms where
the network and host representation of multibyte integers is identical.

It should be replaced with proper tvb_get_xxx() calls for each individual
element to populate
the hdr structure.
hdr should probably then be declared as a structure as well instead of as
now a pointer to a structure.