Wireshark · Wireshark-dev: Re: [Wireshark-dev] Bug 8416 - remove C++ incompatibilities from packet-pw-atm.c

[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

Hi,

I already had my reservations against the construction of the PW dissectors, and
this only adds to it.

What I would like to see is the abolishment of the pwc_packet_properties_t type
altogether. This is _not_ an enum.

typedef enum {
	PWC_CW_BAD_BITS03 		= 1 << 0
	,PWC_CW_BAD_PAYLEN_LT_0		= 1 << 1
	,PWC_CW_BAD_PAYLEN_GT_PACKET	= 1 << 2
	,PWC_CW_BAD_LEN_MUST_BE_0	= 1 << 3
	,PWC_CW_BAD_FRAG 		= 1 << 4
	,PWC_CW_BAD_RSV 		= 1 << 5
	,PWC_CW_BAD_FLAGS 		= 1 << 8
	,PWC_CW_BAD_PAYLEN_LE_0		= 1 << 9
	,PWC_CW_BAD_PADDING_NE_0	= 1 << 10
	,PWC_ANYOF_CW_BAD	= PWC_CW_BAD_BITS03
				+ PWC_CW_BAD_PAYLEN_LT_0
				+ PWC_CW_BAD_PAYLEN_GT_PACKET
				+ PWC_CW_BAD_LEN_MUST_BE_0
				+ PWC_CW_BAD_FRAG
				+ PWC_CW_BAD_RSV
				+ PWC_CW_BAD_FLAGS
				+ PWC_CW_BAD_PAYLEN_LE_0
				+ PWC_CW_BAD_PADDING_NE_0
	,PWC_CW_SUSPECT_LM		= 1 << 6
	,PWC_ANYOF_CW_SUSPECT	= PWC_CW_SUSPECT_LM
	,PWC_PAY_SIZE_BAD		= 1 << 7
}
pwc_packet_properties_t;

It's replacement is a list of #define's or const int's (to stay in C++ realm)

#define PWC_CW_BAD_BITS03           (1 << 0)
#define PWC_CW_BAD_PAYLEN_LT_0      (1 << 1)
#define PWC_CW_BAD_PAYLEN_GT_PACKET (1 << 2)
#define PWC_CW_BAD_LEN_MUST_BE_0    (1 << 3)
#define PWC_CW_BAD_FRAG             (1 << 4)
#define PWC_CW_BAD_RSV 	            (1 << 5)
#define PWC_CW_SUSPECT_LM           (1 << 6)
#define PWC_PAY_SIZE_BAD            (1 << 7)
#define PWC_CW_BAD_FLAGS            (1 << 8)
#define PWC_CW_BAD_PAYLEN_LE_0      (1 << 9)
#define PWC_CW_BAD_PADDING_NE_0     (1 << 10)
#define PWC_ANYOF_CW_BAD	    (PWC_CW_BAD_BITS03 |
                                     PWC_CW_BAD_PAYLEN_LT_0 |
                                     PWC_CW_BAD_PAYLEN_GT_PACKET |
                                     PWC_CW_BAD_LEN_MUST_BE_0 |
                                     PWC_CW_BAD_FRAG |
                                     PWC_CW_BAD_RSV |
                                     PWC_CW_BAD_FLAGS |
                                     PWC_CW_BAD_PAYLEN_LE_0 |
                                     PWC_CW_BAD_PADDING_NE_0)
#define PWC_ANYOF_CW_SUSPECT	    PWC_CW_SUSPECT_LM


This would impact packet-pw-cesopsn.c, packet-pw-satop.c in a similar way as it
did packet-pw-atm.c

Thanks,
Jaap


On 02/28/2013 09:57 PM, Ed Beroset wrote:
> As mentioned in the subject line, I've added Bug 8416 - "remove C++ incompatibilities from packet-pw-atm.c" with the associated patch.  Doing a little forensic work on the C++ incompatibilities still present in the code base, here are the types of issues of the 4919 "c++-incompat" lines in a compilation of the latest source using gcc on a Linux box (Fedora 17) (before this patch):
> 
> type	count 	percent
> implicit_casts	4013	81.58%
> keyword_use	634	12.89%
> enum_conversion	197	4.00%
> uninit_const	7	0.14%
> field_typedef	5	0.10%
> special_operator	3	0.06%
> incompat_ptr	2	0.04%
> other	58	1.18%
> 
> It's clear that the vast majority of these (over 98%) are of only three different kinds which are mostly trivial fixes.  I do want to point out, however, that the way I chose to resolve the enum_conversion complaint was to change the type of one member of a struct from an enum type to an int.  The longer version of the rationale is in the bug report.  If we find this kind of patch acceptable, and desirable, I'll do more.  
> 
> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8416
> 
> Ed
>