Wireshark-bugs: [Wireshark-bugs] [Bug 5929] New protocol dissector for "CIP Motion"
Date: Fri, 16 Sep 2011 07:32:11 -0700 (PDT)

--- Comment #11 from Michael Mann <[email protected]> 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.