https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3922
Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jeff.morriss.ws@xxxxxxxxx
--- Comment #2 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2009-09-17 18:45:58 PDT ---
First: Holy bitmasks, Batman! I'm bet you're glad proto_tree_add_bitmask()
exists... :-)
Second, checkhf.pl found a few hf_ entries that aren't used (among many false
positives). How I found the real problems was to look for entries checkhf
found that only had 2 references in the file:
$ for hf in $(tools/checkhf.pl epan/dissectors/packet-tn5250.c |awk '{print
$4;}'); do printf "$hf "; grep -c $hf epan/dissectors/packet-tn5250.c;
done|grep 2$
hf_tn5250_res_twobytes 2
hf_tn5250_wssf_cc_flag1_7 2
hf_tn5250_character_code 2
hf_tn5250_stop_address 2
hf_tn5250_dpt_id 2
hf_tn5250_wdsf_class 2
hf_tn5250_null 2
hf_tn5250_wdsf_type 2
Third, I saw a comment saying something about checking for memory leaks. Well,
I found two:
+ if (tn5250_info == NULL) {
+ /* No. Attach that information to the conversation, and add
+ * it to the list of information structures.
+ */
+ tn5250_info = g_malloc(sizeof(tn5250_conv_info_t));
+ COPY_ADDRESS(&(tn5250_info->outbound_addr),&(pinfo->dst));
The g_malloc()'d memory is never freed (e.g., when the file is closed) and
neither is the address memory (COPY_ADDRESS allocates memory too). Look at
other dissectors who use the conversation API on how/when to do this.
Fourth, the reliance on tvb_offset_exists() and tvb_length_remaining() mean
that this dissector won't throw exceptions if the packets are cut short because
a snaplen was used when capturing. I suppose a tvb_reported_offset_exists()
could be written, but another way would be to simply check if
tvb_reported_length_remaining() was 0.
Lastly (since you provided a patch to Makfile.common): the header needs to be
in DISSECTOR_INCLUDES. (Not that that this thing really needs a header file
other than to split up the 7kloc.)
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
