Wireshark-dev: [Wireshark-dev] bug 5175 PPI-GEOLOCATION patch, input pcap
From: johnny cache <johnycsh@xxxxxxxxx>
Date: Mon, 8 Nov 2010 15:15:42 -0600
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 ----
- Prev by Date: [Wireshark-dev] bug 5175 PPI-GEOLOCATION patch, input pcap
- Next by Date: [Wireshark-dev] usbmon: size of different fields?
- Previous by thread: Re: [Wireshark-dev] bug 5175 PPI-GEOLOCATION patch, input pcap
- Next by thread: [Wireshark-dev] usbmon: size of different fields?
- Index(es):
- Get Wireshark
- Download
- Code of Conduct