https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=4992
--- Comment #12 from Bill Meier <wmeier@xxxxxxxxxxx> 2010-10-26 14:11:17 EDT ---
OK: A few comments
1. Gearman looks like a fairly simple request/response protocol.
I get the impression (from the protocol spec and from the sample capture)
that any particular Gearman message will only have a single request
or reply.
Is this correct ?
If so, I suggest you start out by simplifying your
code to reflect the above.
That is: in dissect_binary_packet it seems to me that the following kind
of code isn't needed:
while (tvb_reported_length_remaining(tvb, offset) >= ....
and
if (offset == 0)
{
...
}
else
{
...
}
and so on.
Also: in general you do not need to keep checking if there's enough
bytes; Just access the fields as needed. If you run off the
end of the data, Wireshark will throw an exception and show
a "Malformed" message in the tree.
IOW: you don't need to do stuff like:
if (tvb_length_remaining(tvb, offset) >= ...
Note: It presumably can be the case that a Gearman message
can be longer than 1 packet can contain.
Wireshark has a simple mechanism to handle this (for TCP).
The code to handle this case can be added to your dissector
quite simply once all other issues have been addressed.
2. match_strval will return NULL if the lookup fails.
In your code, this will result in %s format being passed a NULL
which is "not a good thing". Use val_to_str().
See doc/README.developer for details.
3. All functions other than proto_register... and proto_reg_handoff...
and all variables should be defined as static.
4. tvb_get_string does a g_malloc; Your usage of same results in memory
leaks since the space allocated is never freed.
Use tvb_get_ephemeral_string().
Again: see doc/README.developer for details
I expect there are additional issues, but the above list is a start. :)
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
