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.)