Re:  https://code.wireshark.org/review/c/34169/

I've simply expanding definitions for existing flags in CDP packets,
specifically for four additional CDP capabilities, yet the Ubuntu
pre-commit check is failing due to code that shouldn't have anything to do
with changes I made.

epan/dissectors/packet-cdp.c:  FT_UINT8:
proto_tree_add_item(tlv_tree, hf_cdp_data, tvb, offset + 4, 2,
[[ENC_BIG_ENDIAN]-->[ENC_NA]]);

It is not a descriptive message at all and it isn't entirely clear what the
issue is. I saw this same issue when trying to commit, myself, but I simply
skipped the check since I don't believe its an issue.

To complicate matters, the "FT_UINT8" seemed to change each time I
attempted to commit, myself; it would be UINT8, UINT16, UINT32, BOOLEAN,
BYTES, STRINGZ, IPV4.. and without knowing what exactly the error is
supposed to be, it seems tedious to track down what the issue is.

I've had a bit of a search and it seems, years ago, there was a mass change
to using ENC_NA everywhere.. which is perhaps what the error is trying to
suggest. Indeed, changing line 620 to use ENC_NA fixes this I believe - but
this shouldn't need to be changed, since it has remained this way for 2
years..

Additionally, it seems there was a legitimate reason why this is
ENC_BIG_ENDIAN, although I'm kind of curious to see examples (@Guy Harris,
do you have any example captures where you found this to be the case?). I'd
be happy to pick that up, but the point remains that I don't believe that
this should be causing an issue.

if (length == 6) {
                    /*
                     * This is some unknown value; it's typically 0x20 0x00,
                     * which, as a big-endian value, is not a VLAN ID, as
                     * VLAN IDs are 12 bits long.
                     */
                    proto_tree_add_item(tlv_tree, hf_cdp_data, tvb, offset
+ 4, 2, ENC_BIG_ENDIAN);


- Oliver
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Reply via email to