Re: Bug 8416 - remove C++ incompatibilities from packet-pw-atm.c

[Jaap Keuter <jaap.keuter () xs4all nl>]

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

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe