Comment # 6
on bug 8594
from Jalil Moraney
Thanks guys, changes will be done and re uploaded.
(In reply to comment #5)
> (In reply to comment #4)
> > Hi Jalil, in general it looks fine.
> >
> > However, I noticed you do a lot of proto_tree_add_bool and
> > proto_tree_add_uint with data retrieved unmodified from the tvb. In this
> > case it is faster and more efficient to simply use proto_tree_add_item and
> > let Wireshark handle fetching the data. For those fields that are only a few
> > bits you can add a bitmask to the hf array entries instead of manually
> > masking in the dissector code.
> >
> + 1
>
> Also I have some warning when i launch Wireshark with your sample
> Warn Extended value string frame_type_subtype_vals forced to fall back to
> linear search: entry 24, value 23 < previous entry, value 362
> Warn Extended value string tag_num_vals forced to fall back to linear
> search: entry 129, value 143 < previous entry, value 221
> Warn Extended value string category_codes forced to fall back to linear
> search: entry 19, value 16 < previous entry, value 127
>
>
> Also checkhf is not happy
> Unused entry: epan/dissectors/packet-ieee80211.c,
> hf_ieee80211_ff_sswf_dmg_select
> Unused entry: epan/dissectors/packet-ieee80211.c,
> hf_ieee80211_ff_sswf_sector_select
> Unused entry: epan/dissectors/packet-ieee80211.c,
> hf_ieee80211_ff_sswf_snr_report
>
> Also why readd some static const true_false_string (dsss_ofdm_flags,
> cf_apsd_flags... )?
>
> I have some warning also with clang analyser tools
> packet-ieee80211.c:13843:11: warning: Value stored to 'offset' is never read
> packet-ieee80211.c:13832:17: warning: Value stored to 'offset' is never read
> packet-ieee80211.c:13848:4: warning: Value stored to 'offset' is never read
> packet-ieee80211.c:13862:11: warning: Value stored to 'offset' is never read
> packet-ieee80211.c:13855:4: warning: Value stored to 'offset' is never read
>
> Check also your indent (the file use 2 spaces indent) and there is some
> trailing whitespaces...
> (From Git : warning: squelched 114 whitespace errors, warning: 119 lines add
> whitespace errors. )
>
> About allocation_duration_base_custom function, use
> beacon_interval_base_custom function (rename if it is needed)
>
> Also there is some typo error : Changing control frame extension for
> extesion frames */ or + {"Low Power SC PHY Supported",
> "wlan.dmg_capa.low_power_suuported", or +
> add_tag_relay_capableities(tag_len, tree, tvb, &offset);
>
> About /* causes a segmentation fault when called. */, you need to
> "initialize" g_pinfo in dissect_dmg_beacon
> @@ -7569,6 +7568,8 @@ dissect_dmg_beacon(packet_info *pinfo, proto_tree
> *tree, tvbuff_t *t
> int tagged_parameter_tree_len;
> int current_offset = offset;
>
> + g_pinfo = pinfo;
You are receiving this mail because:
- You are watching all bug changes.