Hi Everyone,
Just thought I would point out that I uploaded a patch with 3 new
dissectors. Steven had reviewed my first pass at the GPS dissector
previously. I have addressed his comments inline below. I hope
everything is in order. Please let me know if I need to make any changes
for it to be merged.
Thanks,
-Jon E
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);
Fixed.
>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.
Point made, however I would like to keep a copy of the values locally
for future improvements (maybe making a cleaner text representation with
the appropriate
degrees of precision displayed). Also, if I were to skip the
tvb_get_letohl() call,
how do I make sure values are byte-swapped on big-endian systems?
>Don't use g_ as a prefix on things as that is used by GLib and could cause
>confusion.
Fixed.
>Wireshark code typically formats the braces a little differently:
>if(condition) {
> code;
>}
I ran the code through a prettifier before generating the patch. It has
these style brackets now. If there are any particular
indent/astyle/whatever formatter rules you'd like me to run it through
that is fine with me. I just ran astyle --brackets=attach.
I would still like to pursue breaking PPI stuff out with a dissector
table. However I wasn't able to get to it yet, and I'd like to minimize
the amount of changes to existing code I make while committing my own
dissectors. I may get some more PPI tags shortly, that would be a good
opportunity for me to make the change.
Also, I re-ran the fuzzer (with all dissectors loaded and with a input
that triggered every field to begin with)
No events after 7k passes.
----
Starting pass 7176:
/home/johnycsh/ppi_sdk/out.pcap: OK
Starting pass 7177:
/home/johnycsh/ppi_sdk/out.pcap: ^C
----
