Wireshark mailing list archives

Re: Adding data parameter to dissector handler


From: Guy Harris <guy () alum mit edu>
Date: Mon, 3 Sep 2012 18:00:06 -0700


On Sep 3, 2012, at 4:35 PM, Jakub Zawadzki wrote:

I plan to replace:
 typedef void (*dissector_t)(tvbuff_t *, packet_info *, proto_tree *);
 typedef int (*new_dissector_t)(tvbuff_t *, packet_info *, proto_tree *);

with:
 typedef int (*real_dissector_t)(tvbuff_t *, packet_info *, proto_tree *, void *data);

"Replace"?  That means changing *all* dissectors to have the new function signature (not that doing so is the wrong 
thing to do; it's just going to be a lot of work, even if it's work with a shell script or something such as that).

Did you mean "I plan to add to ... this", i.e. keep dissector_t and new_dissector_t around, and add real_dissector_t 
and have new routines with which "new new style" dissectors can register themselves, with older dissectors not being 
passed the additional argument when called through call_dissector(), etc.?

Don't forget to change heur_dissector_t - or add real_heur_dissector_t - and the calls to invoke a heuristic dissector.

call_dissector takes extra parameter data, prototype:
 call_dissector_only(dissector_handle_t handle, tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data);

(assuming we can break this API, if not s/_only/only_data/)

In a major release we can break any APIs we want, as long as we do enough of what somebody at NetApp called "elfware", 
i.e. "elves" fix up all the code as part of the process of changing the API.

That means also changing the dissector_try_ APIs, of course.  Those changes don't involve "leaf node" dissectors (i.e., 
dissectors that never call other dissectors), so they wouldn't be quite as large a project as changing the function 
signature of all dissectors (changing those could be done over time).

Or new new versions of those APIs could be added, with the current versions passing NULL as the extra data, and the new 
new versions used in dissectors currently passing data through pinfo->private_data (again, the other dissectors could 
be changed over time).

Rationale:
 Right now pinfo structure and pinfo->private_data can be used to pass data from one dissector to another.

 Extending packet_info is considered as bad idea[1], and using pinfo->private_data is PITA (you need to restore it 
after changing).
 New API with passing data looks much cleaner and should be easies to use.

Sounds good.

*If* we could come up with a dissector function signature that separates the "telling the caller whether we think this 
packet is one of our packets or not" function from the "telling the caller how much packet data we dissected" function 
(I'd have to try to find it, but there *were* reasons why I couldn't just make all dissectors new_dissector_t-style 
dissectors; I *think* it involved dissectors for replies in protocols where the containing protocol had a reply status 
variable and error replies could be zero-length), and do that at the same time, that would be nice.

Is it worth also having dissectors take a ptvcursor as an argument?  (That might solve the previous problem - the 
amount of data dissected by the caller is the difference between the initial position in the ptvcursor and the position 
after the dissection is done, and the dissector could just return a gboolean indicating whether it accepted the packet 
or not.)

___________________________________________________________________________
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


Current thread: