Wireshark-dev: [Wireshark-dev] Return value of a new-style dissector

From: Peter Wu <peter@xxxxxxxxxxxxx>
Date: Wed, 25 Jun 2014 18:32:44 +0200
Hi,

Since Pascal's change ("TCP: do desegmentation sanity checks for all sub 
dissectors types"), the whois dissector was starting to throw:

Dissector bug, protocol TCP, in packet 6: epan/dissectors/packet-tcp.c:3953: 
failed assertion "save_desegment_offset == pinfo->desegment_offset && 
save_desegment_len == pinfo->desegment_len"

That came from this part:

    pinfo->desegment_len = DESEGMENT_UNTIL_FIN;
    pinfo->desegment_offset = 0;
    return (0);

It is likely supposed to mean "well, this packet is mine, please give all data 
until the connection is closed". The `return 0` is clearly wrong here. But
what is the correct value?

>From epan/packet.h:
/*
 * Dissector that returns:
 *
 *	The amount of data in the protocol's PDU, if it was able to
 *	dissect all the data;
 *
 *	0, if the tvbuff doesn't contain a PDU for that protocol;
 *
 *	The negative of the amount of additional data needed, if
 *	we need more data (e.g., from subsequent TCP segments) to
 *	dissect the entire PDU.
 */
typedef int (*new_dissector_t)(tvbuff_t *, packet_info *, proto_tree *, void *);

Almost all callers of call_dissector... ignore its return value, those who
care seem only to be interested in a boolean (zero vs non-zero). The dissectors
which use DESEGMENT_... do arbitrary things:

 - Return 0, this is just a plain error. See whois, finger, snmp, alljoyn, ms-mms.
 - Return tvb_length(tvb). See nbns
 - Return -pinfo->desegment_len. See ldp.
 - Return -1. See sigcomp, rtsp
 - Return -2. See fcip.
 - Return offset. See ssh, ssl
 - Return -DESEGMENT_ONE_MORE_SEGMENT (!!). jxta
 - Return the actual bytes that is wanted (offset_next - offset_before). See xot.

(Found with `view -p $(grep -rnl DESEGMENT_ONE_MORE_SEGMENT)`)

This suggests that something is wrong in the API definition. As it stands now,
it really needs a boolean. Can someone clarify this? Is this a bug, an
incomplete migration from the old-style dissector or a documentation issue?
Did I misunderstood something?

Kind regards,
Peter