[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6787

--- Comment #14 from Bill Meier <wmeier@xxxxxxxxxxx> 2012-07-26 14:23:29 PDT ---
FF:

There are two cases (see below) wherein the dissector does a 'return 0' which
seem incorrect to me. (The 3rd case of 'return 0' is redundant).

packet-mpls.c treats a return value of 0 as "not dissected" (not found ?)
and then continues with other tries at dissection.

However, packet-mpls-y1711.c acts as "found" even though doing a 'return 0'.
That is, data is added either to the info column or to the tree and thus,
presumably, packet-mpls need not continue to try to dissect.

(Not the most earthshaking issue), but what do you think ? (See below for my
suggested changes).


The existing code;
    if (!tree) {
        if (check_col(pinfo->cinfo, COL_INFO)) {
            if (tvb_bytes_exist(tvb, offset, 1)) {
                functype = tvb_get_guint8(tvb, offset);
                col_append_fstr(pinfo->cinfo, COL_INFO, " (Y.1711: %s)",
                                (functype == 0x01) ? "CV" :
                                (functype == 0x02) ? "FDI" :
                                (functype == 0x03) ? "BDI" :
                                (functype == 0x07) ? "FDD" :
                                "reserved/unknown");
            }
        }
        return 0;
    }

    /* sanity checks */
    if (!tvb_bytes_exist(tvb, offset, 44)) {
        /*
         * ITU-T Y.1711, 5.3: PDUs must have a minimum payload length of
         * 44 bytes
         */
        proto_tree_add_text(tree, tvb, offset, -1,
                            "Error: must have a minimum payload "
                            "length of 44 bytes");
        return 0;
    }

==============

I would be inclined to do something the following:

    functype = tvb_get_guint8(tvb, offset);  // will do bounds error if no data
    col_append_fstr(pinfo->cinfo, COL_INFO, " (Y.1711: %s)",
                    (functype == 0x01) ? "CV" :
                    (functype == 0x02) ? "FDI" :
                    (functype == 0x03) ? "BDI" :
                    (functype == 0x07) ? "FDD" :
                    "reserved/unknown");

    /* sanity checks */
    if (tvb_reported_length(tvb) < 44)) {
        /*
         * ITU-T Y.1711, 5.3: PDUs must have a minimum payload length of
         * 44 bytes
         */
        proto_tree_add_text(tree, tvb, offset, -1,
                            "Error: must have a minimum payload "
                            "length of 44 bytes");
        <call the data dissector to display (some of) the bytes>

        return tvb_reported_length(tvb);
    }

    if (!tree)
        return 

    ...

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.