Wireshark mailing list archives

Re: Actualize kafka dissector


From: Michael Mann <mmann78 () netscape net>
Date: Mon, 14 Nov 2016 21:32:27 -0500


The answer is typically "follow the coding convention of the dissector if modifying an existing dissector".  And when 
most dissectors are created, they start by using the example packet-PROTOABBREV.c (which explicitly lists out the 
parameters).  I don't think the performance is an issue either way.
 
The Wireshark dissector architecture is very conducive to copy/paste, which also perpetuates the convention of 
explicitly listing out the parameters (so proto_tree_add_xxx functions only need very minimal changes if copying from 
another dissector).  This is also why I don't think it's "extra typing" to list the parameters out because you're 
probably just doing from copy/paste shortcut keys.

Wireshark does have a ptvcursor API that can save you some of the "standard" function parameters, but based on your 
example, I don't think it'll help that much.

 
 
-----Original Message-----
From: Dmitry Lazurkin <dilaz03 () gmail com>
To: Developer support list for Wireshark <wireshark-dev () wireshark org>
Sent: Mon, Nov 14, 2016 6:04 pm
Subject: Re: [Wireshark-dev] Actualize kafka dissector

Second question about using "API version" value in dissect functions. 
This value may change parsing of packet. For now it passed as argument 
to some dissect functions. But for now it used only in root dissect 
function of request/response type (not passed to nested functions). I 
have next solutions for this:

---------------------------------------
Use current solution: always pass api_version variable to all functions 
and subfunctions.

Too many arguments for all functions. May be this is normal (:

---------------------------------------
Create context structure and pass it to functions/subfunctions:

typedef struct _kafka_context_t {
     tvbuff_t *tvb;
     packet_info *pinfo;
     guint16 api_version;
     int offset;
} kafka_context_t;

offset field is not necessary. Create instance of this struct in root 
dissect function as local variable and pass it to dissect functions by 
pointer. Use context like this:

proto_tree_add_item(tree, hf_kafka_string_len, ctx->tvb, ctx->offset, 2, 
ENC_BIG_ENDIAN);

Here i have question. Are many pointer dereferences perfomance issue?

---------------------------------------

Create separate function for parsing each version of packet with code 
duplication. Too much code but may be better for supporting.


I like context but passing version as separate argument is good too. 
Which solution is better in wireshark community?


___________________________________________________________________________
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-request () wireshark org?subject=unsubscribe

___________________________________________________________________________
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-request () wireshark org?subject=unsubscribe

Current thread: