https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5175
--- Comment #1 from Stephen Fisher <steve@xxxxxxxxxxxxxxxxxx> 2010-09-15 09:45:34 MDT ---
Can you make some minor changes:
The following line from packet-ppi-gps.c doesn't need the (guint32) cast since
tvb_get_letohl() already returns a guint32 (despite its historical use of the
"long"): fixtype_flags = (guint32) tvb_get_letohl(tvb, offset);
Is there a reason g_appstr_num is fetched using tvb_get_letohl() and then added
to the tree with proto_tree_add_uint() instead of adding it directly with
proto_tree_add_item()? That would make for cleaner code as it appears that
g_appstr_num isn't used after the proto_tree_add_uint() call. using
_add_item() makes the code easier to read if the variable isn't needed after
that. There may be other cases of this; this is just an example I noticed.
Don't use g_ as a prefix on things as that is used by GLib and could cause
confusion.
Please make the unified diff relative to the base Wireshark directory (so the
file names have the path of epan/dissectors in front of them) to make it easier
on us :).
It's quite a comprehensive addition to the PPI header.. it may be worth using a
sub-dissector registration table to make adding hooks into the main ppi
dissector even easier.
Wireshark code typically formats the braces a little differently:
if(condition) {
code;
}
Otherwise, it looks fine.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
