Re: [Wireshark-dev] [Patch] pragma warning

2007-03-27 Thread Graham Bloice
Gisle Vanem wrote:
> The "#pragma warning()" statements are MSVC specific. So
> embed them inside "#ifdef _MSC_VER".
> 

I always thought unrecognised #pragma directives were ignored.  I take
it that MinGw barfs on them in some way?

-- 
Regards,

Graham Bloice
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-27 Thread Gisle Vanem
"Graham Bloice" <[EMAIL PROTECTED]> wrote:

> Gisle Vanem wrote:
>> The "#pragma warning()" statements are MSVC specific. So
>> embed them inside "#ifdef _MSC_VER".
>> 
> 
> I always thought unrecognised #pragma directives were ignored.  I take
> it that MinGw barfs on them in some way?

MingW ignores them, but with a warning. I thought we should strive
to reduce the number of warnings. Hence my patch.

--gv
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-27 Thread Ulf Lamping
Gisle Vanem wrote:
> The "#pragma warning()" statements are MSVC specific. So
> embed them inside "#ifdef _MSC_VER".
>
> Patch attached.
>
checked in,

If you still have such problems, please note.

Regards, ULFL
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-27 Thread Gisle Vanem
"Ulf Lamping" <[EMAIL PROTECTED]> wrote:

> If you still have such problems, please note.

There's still some in generated files. It seems someone added the pragmas after 
they where autogenerated. IMHO the #pragma should go elsewhere. But
where, should be left to the experts.

A patch for now:

diff -u3 -Hb -r SVN-Latest\epan\dissectors\packet-dcerpc-dfs.c 
.\epan\dissectors\packet-dcerpc-dfs.c
--- SVN-Latest\epan\dissectors\packet-dcerpc-dfs.c Tue Mar 27 17:18:50 2007
+++ .\epan\dissectors\packet-dcerpc-dfs.c Tue Mar 27 17:44:53 2007
@@ -22,7 +22,7 @@
 #include "packet-windows-common.h"
 #include "packet-dcerpc-dfs.h"
 
-#ifdef _WIN32
+#ifdef _MSC_VER
 /* disable: warning C4101: 'xy' : unreferenced local variable */
 #pragma warning(disable:4101)
 #endif
diff -u3 -Hb -r SVN-Latest\epan\dissectors\packet-dcerpc-eventlog.c 
.\epan\dissectors\packet-dcerpc-eventlog.c
--- SVN-Latest\epan\dissectors\packet-dcerpc-eventlog.c Tue Mar 27 17:18:52 2007
+++ .\epan\dissectors\packet-dcerpc-eventlog.c Tue Mar 27 17:45:04 2007
@@ -22,7 +22,7 @@
 #include "packet-windows-common.h"
 #include "packet-dcerpc-eventlog.h"
 
-#ifdef _WIN32
+#ifdef _MSC_VER
 /* disable: warning C4018: '<' : signed/unsigned mismatch */
 #pragma warning(disable:4018)
 #endif

diff -u3 -Hb -r SVN-Latest\plugins\giop\packet-cosnaming.c 
.\plugins\giop\packet-cosnaming.c
--- SVN-Latest\plugins\giop\packet-cosnaming.c Tue Mar 27 17:18:48 2007
+++ .\plugins\giop\packet-cosnaming.c Wed Mar 28 05:20:33 2007
@@ -48,7 +48,7 @@
 G_MODULE_EXPORT const gchar version[] = "0.0.1";
 #endif
 
-#ifdef _WIN32
+#ifdef _MSC_VER
 /* disable warning: "unreference local variable" */
 #pragma warning(disable:4101)
 #endif

And for airpcap.h:

diff -u3 -Hb -r SVN-Latest\airpcap.h .\airpcap.h
--- SVN-Latest\airpcap.h Tue Jan 02 23:26:52 2007
+++ .\airpcap.h Wed Mar 28 03:49:39 2007
@@ -24,8 +24,10 @@
 
 #include   /* WEP_KEY_MAX_SIZE */
 
+#ifdef _MSC_VER
 /* This disables a VS warning for zero-sized arrays. All the compilers we 
support have that feature */
 #pragma warning( disable : 4200)
+#endif
 
 #ifdef __cplusplus
 extern "C" {

---
--gv
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-27 Thread Ulf Lamping
Gisle Vanem wrote:
> "Ulf Lamping" <[EMAIL PROTECTED]> wrote:
>
>   
>> If you still have such problems, please note.
>> 
>
> There's still some in generated files. It seems someone added the pragmas 
> after 
> they where autogenerated. IMHO the #pragma should go elsewhere. But
> where, should be left to the experts.
>
>   
Checked in your changes.

I've added the pragma's a few days ago. I've asked about how to fix this 
cleanly, but just got no response ...

Regards, ULFL
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-27 Thread Graham Bloice
Gisle Vanem wrote:
> "Graham Bloice" <[EMAIL PROTECTED]> wrote:
> 
>> Gisle Vanem wrote:
>>> The "#pragma warning()" statements are MSVC specific. So
>>> embed them inside "#ifdef _MSC_VER".
>>>
>> I always thought unrecognised #pragma directives were ignored.  I take
>> it that MinGw barfs on them in some way?
> 
> MingW ignores them, but with a warning. I thought we should strive
> to reduce the number of warnings. Hence my patch.
> 

"Ignore" and "issues a warning" don't quite mesh in my dictionary, but
now I understand what's up I support your solution.

-- 
Regards,

Graham Bloice

___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-28 Thread ronnie sahlberg
I dont think it is really realistic to have all autogenerated files
always compile without any warnings.
Maybe we should instead split Makefile.common up into three parts :

First part : normal dissectors

Second part : ANS2WRS generated dissectors  which take extra compile
time flags and definitions to suppress artefacts from the compiler.

Third part : PIDL generated dissectors that once again take extra
compile parameters and definitions.




On 3/28/07, Graham Bloice <[EMAIL PROTECTED]> wrote:
> Gisle Vanem wrote:
> > "Graham Bloice" <[EMAIL PROTECTED]> wrote:
> >
> >> Gisle Vanem wrote:
> >>> The "#pragma warning()" statements are MSVC specific. So
> >>> embed them inside "#ifdef _MSC_VER".
> >>>
> >> I always thought unrecognised #pragma directives were ignored.  I take
> >> it that MinGw barfs on them in some way?
> >
> > MingW ignores them, but with a warning. I thought we should strive
> > to reduce the number of warnings. Hence my patch.
> >
>
> "Ignore" and "issues a warning" don't quite mesh in my dictionary, but
> now I understand what's up I support your solution.
>
> --
> Regards,
>
> Graham Bloice
>
> ___
> Wireshark-dev mailing list
> Wireshark-dev@wireshark.org
> http://www.wireshark.org/mailman/listinfo/wireshark-dev
>
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-28 Thread Ulf Lamping
ronnie sahlberg wrote:
> I dont think it is really realistic to have all autogenerated files
> always compile without any warnings.
> Maybe we should instead split Makefile.common up into three parts :
>
> First part : normal dissectors
>
> Second part : ANS2WRS generated dissectors  which take extra compile
> time flags and definitions to suppress artefacts from the compiler.
>
> Third part : PIDL generated dissectors that once again take extra
> compile parameters and definitions.
>   
Sounds like a reasonable solution. It would also have the advantage that 
we get a list of generated dissectors, which we currently don't have.

But: Is there a "makefile magic", so you can have different CFLAGS 
settings for the different lists? Remember that we are using implicit 
rules here.

Having three lists shouldn't be difficult to add, but I don't know how 
to get the "compile logic" for it - however, I'm really not an expert on 
this.

If there's a solution for this makefile problem, I think it's the way to 
go ...

Regards, ULFL

___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-28 Thread ronnie sahlberg
On 3/28/07, Ulf Lamping <[EMAIL PROTECTED]> wrote:
> ronnie sahlberg wrote:
> > I dont think it is really realistic to have all autogenerated files
> > always compile without any warnings.
> > Maybe we should instead split Makefile.common up into three parts :
> >
> > First part : normal dissectors
> >
> > Second part : ANS2WRS generated dissectors  which take extra compile
> > time flags and definitions to suppress artefacts from the compiler.
> >
> > Third part : PIDL generated dissectors that once again take extra
> > compile parameters and definitions.
> >
> Sounds like a reasonable solution. It would also have the advantage that
> we get a list of generated dissectors, which we currently don't have.
>
> But: Is there a "makefile magic", so you can have different CFLAGS
> settings for the different lists? Remember that we are using implicit
> rules here.
>
> Having three lists shouldn't be difficult to add, but I don't know how
> to get the "compile logic" for it - however, I'm really not an expert on
> this.
>
> If there's a solution for this makefile problem, I think it's the way to
> go ...

When it comes to the ANS2WRS dissectors are concerned  I think that
ANS2WRS is so mature now that maybe what we should do for those
dissectors would instead be to
remove them completely from epan/dissectors and make asn part of the
build process.
All these dissectors have their own makefile down in asn/* where one
can set additional or different compiler flags compared to those used
by the normal epan/dissectors.
These asn dissectors could even be linked to a separate dll :
libwireshark-asn.dll


As far as i remember,  the manual process of "make copy_files" and not
having the asn compilation part of the build process was really just
to wait doing so until we were sure asn2wrs was mature enough.
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-28 Thread Luis Ontanon
On 3/28/07, ronnie sahlberg <[EMAIL PROTECTED]> wrote:
> On 3/28/07, Ulf Lamping <[EMAIL PROTECTED]> wrote:
> > ronnie sahlberg wrote:
> > > I dont think it is really realistic to have all autogenerated files
> > > always compile without any warnings.
> > > Maybe we should instead split Makefile.common up into three parts :
> > >
> > > First part : normal dissectors
> > >
> > > Second part : ANS2WRS generated dissectors  which take extra compile
> > > time flags and definitions to suppress artefacts from the compiler.
> > >
> > > Third part : PIDL generated dissectors that once again take extra
> > > compile parameters and definitions.
> > >
> > Sounds like a reasonable solution. It would also have the advantage that
> > we get a list of generated dissectors, which we currently don't have.
> >
> > But: Is there a "makefile magic", so you can have different CFLAGS
> > settings for the different lists? Remember that we are using implicit
> > rules here.
> >
> > Having three lists shouldn't be difficult to add, but I don't know how
> > to get the "compile logic" for it - however, I'm really not an expert on
> > this.
> >
> > If there's a solution for this makefile problem, I think it's the way to
> > go ...
>
> When it comes to the ANS2WRS dissectors are concerned  I think that
> ANS2WRS is so mature now that maybe what we should do for those
> dissectors would instead be to
> remove them completely from epan/dissectors and make asn part of the
> build process.
> All these dissectors have their own makefile down in asn/* where one
> can set additional or different compiler flags compared to those used
> by the normal epan/dissectors.

Full Ack,

> These asn dissectors could even be linked to a separate dll :
> libwireshark-asn.dll


Wouldn't be better to move trunk/asn1 into trunk/epan/dissectors like
the pidl ones?

>
> As far as i remember,  the manual process of "make copy_files" and not
> having the asn compilation part of the build process was really just
> to wait doing so until we were sure asn2wrs was mature enough.

-- 
This information is top security. When you have read it, destroy yourself.
-- Marshall McLuhan
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-28 Thread Joerg Mayer
On Wed, Mar 28, 2007 at 08:21:24AM +, ronnie sahlberg wrote:
> I dont think it is really realistic to have all autogenerated files
> always compile without any warnings.

Which warnings do you have in mind specifically? Why do you think they
can't be avoided?

> Maybe we should instead split Makefile.common up into three parts :
> 
> First part : normal dissectors
> 
> Second part : ANS2WRS generated dissectors  which take extra compile
> time flags and definitions to suppress artefacts from the compiler.

I think they should just be generated from their "real sources" on each
build, thus removing the need to manually rebuild them at all.

> Third part : PIDL generated dissectors that once again take extra
> compile parameters and definitions.

dito.

 ciao
Joerg
-- 
Joerg Mayer   <[EMAIL PROTECTED]>
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-28 Thread Sebastien Tandel

>>> I dont think it is really realistic to have all autogenerated files
>>> always compile without any warnings.
>>> Maybe we should instead split Makefile.common up into three parts :
>>>
>>> First part : normal dissectors
>>>
>>> Second part : ANS2WRS generated dissectors  which take extra compile
>>> time flags and definitions to suppress artefacts from the compiler.
>>>
>>> Third part : PIDL generated dissectors that once again take extra
>>> compile parameters and definitions.
>>>
>>>   
>> Sounds like a reasonable solution. It would also have the advantage that
>> we get a list of generated dissectors, which we currently don't have.
>>
>> But: Is there a "makefile magic", so you can have different CFLAGS
>> settings for the different lists? Remember that we are using implicit
>> rules here.
>>
>> Having three lists shouldn't be difficult to add, but I don't know how
>> to get the "compile logic" for it - however, I'm really not an expert on
>> this.
>>
>> If there's a solution for this makefile problem, I think it's the way to
>> go ...
>> 
>
> When it comes to the ANS2WRS dissectors are concerned  I think that
> ANS2WRS is so mature now that maybe what we should do for those
> dissectors would instead be to
> remove them completely from epan/dissectors and make asn part of the
> build process.
> All these dissectors have their own makefile down in asn/* where one
> can set additional or different compiler flags compared to those used
> by the normal epan/dissectors.
> These asn dissectors could even be linked to a separate dll :
> libwireshark-asn.dll
>   
Wouldn't be the register_all_protocols() function problematic?


Regards,
Sebastien Tandel
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-28 Thread Luis Ontanon
On 3/28/07, Sebastien Tandel <[EMAIL PROTECTED]> wrote:
>
> >>> I dont think it is really realistic to have all autogenerated files
> >>> always compile without any warnings.
> >>> Maybe we should instead split Makefile.common up into three parts :
> >>>
> >>> First part : normal dissectors
> >>>
> >>> Second part : ANS2WRS generated dissectors  which take extra compile
> >>> time flags and definitions to suppress artefacts from the compiler.
> >>>
> >>> Third part : PIDL generated dissectors that once again take extra
> >>> compile parameters and definitions.
> >>>
> >>>
> >> Sounds like a reasonable solution. It would also have the advantage that
> >> we get a list of generated dissectors, which we currently don't have.
> >>
> >> But: Is there a "makefile magic", so you can have different CFLAGS
> >> settings for the different lists? Remember that we are using implicit
> >> rules here.
> >>
> >> Having three lists shouldn't be difficult to add, but I don't know how
> >> to get the "compile logic" for it - however, I'm really not an expert on
> >> this.
> >>
> >> If there's a solution for this makefile problem, I think it's the way to
> >> go ...
> >>
> >
> > When it comes to the ANS2WRS dissectors are concerned  I think that
> > ANS2WRS is so mature now that maybe what we should do for those
> > dissectors would instead be to
> > remove them completely from epan/dissectors and make asn part of the
> > build process.
> > All these dissectors have their own makefile down in asn/* where one
> > can set additional or different compiler flags compared to those used
> > by the normal epan/dissectors.
> > These asn dissectors could even be linked to a separate dll :
> > libwireshark-asn.dll
> >
> Wouldn't be the register_all_protocols() function problematic?

We could create a plugin directory with all its paraphernalia and have
the asn1 dissectors be loaded like a plugin.

But still the best thing would be to have the asn1 generated
dissectors inepan/dissectors and have them built from the Makefile.

For the warnings we could have asn2wrs.py prepend the #pragma for
unused static function  right bellow the signature to the generated
code (that's an ugly fix but I do not find a feasible way to have
asn2wrs not creating the unused functions).

The rest is monkey business: just to add the generated targets to the
Makefile[s] like this:

packet-h248.c: ../../asn1/h248/packet-h248-template.c \
   ../../asn1/h248/packet-h248-template.h \
   ../../asn1/h248/h248.cnf \
   ../../asn1/h248/h248v3.asn
[TAB](cd ../../asn1/h248 && make copy_files )

packet-h248.h: packet-h248.c


Luis

> Regards,
> Sebastien Tandel
> ___
> Wireshark-dev mailing list
> Wireshark-dev@wireshark.org
> http://www.wireshark.org/mailman/listinfo/wireshark-dev
>


-- 
This information is top security. When you have read it, destroy yourself.
-- Marshall McLuhan
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-03-28 Thread Sebastien Tandel
Hi Ulf,

>> Maybe we should instead split Makefile.common up into three parts :
>>
>> First part : normal dissectors
>>
>> Second part : ANS2WRS generated dissectors  which take extra compile
>> time flags and definitions to suppress artefacts from the compiler.
>>
>> Third part : PIDL generated dissectors that once again take extra
>> compile parameters and definitions.
>>   
>> 
> Sounds like a reasonable solution. It would also have the advantage that 
> we get a list of generated dissectors, which we currently don't have.
>
> But: Is there a "makefile magic", so you can have different CFLAGS 
> settings for the different lists? Remember that we are using implicit 
> rules here.
>
> Having three lists shouldn't be difficult to add, but I don't know how 
> to get the "compile logic" for it - however, I'm really not an expert on 
> this.
>   

I made it partly for the Unix side. (Makefile.common and Makefile.am
affected).
The Makefile is, in fact, building now four libraries :
- asn dissectors : libasndissectors.la
- pidl dissectors : libpidldissectors.la
- normal dissectors : libdissectors.la *and* libcleandissectors.la. I
separated it in two libraries temporarily. The source files used to
build libcleandissectors.la do not generate warning anymore and the
-Werror is used to compile them. If we patch a dissector and it doesn't
generate warning anymore, we have to move the filename dissector from
DISSECTOR_SRC to CLEAN_DISSECTOR_SRC in epan/dissectors/Makefile.common.

You can define specific cflags with, for example,
libpidldissectors_la_CFLAGS.

The problem is  that I really don't know how to do this for Makefile.nmake.
Maybe the attached patch will give you some ideas ;)


Regards,
Sebastien Tandel



sources-per-dissector-type.diff.gz
Description: application/gzip
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-04-02 Thread Stephen Fisher
On Wed, Mar 28, 2007 at 05:01:06PM +0200, Sebastien Tandel wrote:

> I made it partly for the Unix side. (Makefile.common and Makefile.am
> affected).
> The Makefile is, in fact, building now four libraries :
> - asn dissectors : libasndissectors.la
> - pidl dissectors : libpidldissectors.la
> - normal dissectors : libdissectors.la *and* libcleandissectors.la. I
> separated it in two libraries temporarily. The source files used to
> build libcleandissectors.la do not generate warning anymore and the
> -Werror is used to compile them. If we patch a dissector and it doesn't
> generate warning anymore, we have to move the filename dissector from
> DISSECTOR_SRC to CLEAN_DISSECTOR_SRC in epan/dissectors/Makefile.common.
> 
> You can define specific cflags with, for example,
> libpidldissectors_la_CFLAGS.

Did you already commit these changes?  I don't see them in my svn tree.  
We're still compiling epan/dissectors with a ton of warnings from 
auto-generated dissectors on Unix.

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.   
time_t is typically a signed value, but isn't guaranteed to be from what
I've read.  Making the value LL (long long) is only officially supported 
in c99, not the c90 that we code to.  Any ideas on how to fix this in a 
portable fashion?


Steve

 
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-04-02 Thread Guy Harris

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.

> 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.


> time_t is typically a signed value, but isn't guaranteed to be from  
> what
> I've read.

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.


> Making the value LL (long long) is only officially supported
> in c99, not the c90 that we code to.  Any ideas on how to fix this  
> in a
> portable fashion?

64-bit integer support isn't needed for this, but as a general FYI on  
64-bit integer support in Wireshark, gint64_t or guint64_t are  
supported on all the platforms we support (we no longer support  
platforms where the compiler and formatted-output routines don't  
support 64-bit integral data types).
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-04-02 Thread Stephen Fisher
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

___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev


Re: [Wireshark-dev] [Patch] pragma warning

2007-04-03 Thread Sebastien Tandel

Now these changes are in the svn tree.

Note that I've moved some dissectors from DISSECTOR_SRC to
CLEAN_DISSECTOR_SRC but there are many others which could already be
moved to CLEAN_DISSECTOR_SRC.


Regards,
Sebastien Tandel

>> I made it partly for the Unix side. (Makefile.common and Makefile.am
>> affected).
>> The Makefile is, in fact, building now four libraries :
>> - asn dissectors : libasndissectors.la
>> - pidl dissectors : libpidldissectors.la
>> - normal dissectors : libdissectors.la *and* libcleandissectors.la. I
>> separated it in two libraries temporarily. The source files used to
>> build libcleandissectors.la do not generate warning anymore and the
>> -Werror is used to compile them. If we patch a dissector and it doesn't
>> generate warning anymore, we have to move the filename dissector from
>> DISSECTOR_SRC to CLEAN_DISSECTOR_SRC in epan/dissectors/Makefile.common.
>>
>> You can define specific cflags with, for example,
>> libpidldissectors_la_CFLAGS.
>> 
>
> Did you already commit these changes?  I don't see them in my svn tree.  
> We're still compiling epan/dissectors with a ton of warnings from 
> auto-generated dissectors on Unix.
>   
___
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev