Re: [Wireshark-dev] [Patch] pragma warning
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
Re: [Wireshark-dev] [Patch] pragma warning
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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 epan/crypt/wep-wpadefs.h /* 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
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
