Wireshark-bugs: [Wireshark-bugs] [Bug 5929] New protocol dissector for "CIP Motion"
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5929
--- Comment #11 from Michael Mann <mmann78@xxxxxxxxxxxx> 2011-09-16 07:32:08 PDT ---
(In reply to comment #9)
> Michael: I guess you've updated the patch on behalf of Benjamin Stocks.
I know enough about CIP to be dangerous. I haven't gotten involved with
CIP-Motion yet, but it would be nice to have it availble in Wireshark by then.
Besides learning about the CIP-Motion dissector gave me a few pointers on how
to improve other dissectors (ie CIP).
> > Mixed tab/multi space indentation has to go.
My biggest frustration is it seems each dissector I work with seems to have a
different whitespace rule. I'd like to honor the "keep style as is", but
sometimes it goes unnoticed when developing if tab space value = # of spaces
used. I'll fix it.
> I'd start with:
> cip_gs_vals
> Speaking of which:
> cip_gs_vals in packet-cipmotion.c is the same as that in packet-cip.c except
> that the last two entries are missing. Do you know: is this correct or should
> the value_string arrays be identical ? If they are identical, then please use
> one common value_string; (actually just the extended value string pointer would
> need to be public).
Wasn't sure if "public" variable or essentially recreating the value string was
the way to go (and have that "common" code (i.e. large macro) in packet-cip.h).
In all likelihood they would remain the same.
> 12) checkAPIs.pl finds the following problems:
> $ tools/checkAPIs.pl epan/dissectors/packet-cipmotion.c
> Warning: epan/dissectors/packet-cipmotion.c does not have an SVN Id tag.
> $Id$ still missing;
I thought this was added at checkin time (automatically through SVN), and since
the dissector is new, wouldn't that fall on the developer doing checkin?
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
You are watching all bug changes.
