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
>
