Wireshark-dev: Re: [Wireshark-dev] [Patch] pragma warning
From: Stephen Fisher <[email protected]>
Date: Mon, 2 Apr 2007 17:42:16 -0700
On Mon, Apr 02, 2007 at 04:40:05PM -0700, Guy Harris wrote:
> 
> On Apr 2, 2007, at 4:13 PM, Stephen Fisher wrote:
> 
> > We're still compiling epan/dissectors with a ton of warnings from
> > auto-generated dissectors on Unix.
> 
> How many of them are coming from asn2wrs-generated dissectors?
>
> asn2wrs is, for some reason, generating a lot of dissect_ functions 
> that aren't being called.
>
> I think it might also be generating some hf_ values that aren't being 
> used, although I might be thinking of some other set of generated 
> dissectors with that problem.

Assuming that they have either template, -fn, or .cnf in their names 
when compiling, almost all of the remaining warnings come from the ASN1 
dissectors.  There are 3,265 warnings from these files.  All but about 
20 are "defined but not used" warnings (only 3 of which include hf_ in 
the name - the rest start with dissect_).

> > There is only one warning left from the regular dissectors and I'm not
> > quite sure how to fix it properly:
> >
> > packet-diameter.c: In function `dissect_avps':
> > packet-diameter.c:2057: warning: comparison between signed and  
> > unsigned
> >
> > The relevant code is:
> >
> >        if ( data.secs >= NTP_TIME_DIFF){
> >
> > Where data is a nstime_t (and secs is a time_t).  NTP_TIME_DIFF is
> > defined as 2208988800UL since it's too large to be a signed int/long.
> 
> We don't support platforms on which "int" is shorter than 32 bits, so 
> 2208988800U *should* suffice, unless there's something I'm missing.

Good point, I have changed that.

> According to RFC 2030 (which defines the NTP time stamp format also 
> used in Diameter), the NTP time stamp is unsigned.  Therefore, the 
> seconds field of an NTP time stamp should be fetched into a guint32_t 
> value.
> 
> Then, the relevant code would be
> 
> 	if (ntp_timestamp_secs >= NTP_TIME_DIFF) {
> 
> which is comparing two unsigned values.
> 
> Inside that branch of the if, I'd do
> 
> 		data.secs = ntp_timestamp_secs - NTP_TIME_DIFF;
> 
> with "data" being the nstime_t.

Thanks!  No more warning :).  I'll commit it with my next batch of 
warnings fixes - some more have crept in since I upgraded from gcc 4.0 
to 4.1.


Steve