[bugzilla-daemon@xxxxxxxxxxxxx]

Alexis La Goutte changed bug 8594

What	Removed	Added
Status	IN_PROGRESS	INCOMPLETE

Comment # 10 on bug 8594 from Alexis La Goutte

(In reply to comment #7)

> 
> > Also why readd some static const true_false_string (dsss_ofdm_flags,
> > cf_apsd_flags... )? 
> 
> Please further explain of what you think the proper behaviour should be.
> 
Fixed by Michael

> > 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
> 
> We didn't get the warnings after the changes, however please verify they are
> gone since we don't have any prior experience with clang tools.
> 
I have always this warning with Clang (the offset value is never read after...)

(In reply to comment #9)
> In addition:
> Some of the TAG_DMG_* cases appear to have overlapping offsets.  Should the
> boolean values really be 16 bits?
> 
> TAG_DMG_OPERATION (and some others) seems to have a bit field of 16 bytes,
> which would make the items 2 bytes, not 1.

I confirm 
It is not possible proto_tree_add_bitmask ? or use the some length for each hf
? (it is not easy to read)
Example for Antenna Sector ID with your patch :
        Tag: Antenna Sector ID
            Tag Number: Antenna Sector ID (190)
            Tag length: 4
            .... 0000 = Type: 0x00
            .... ..00 0000 .... = Tap 1: 0x0000
            0000 00.. = State 1: 0x00
            0000 0000 = Tap 2: 0x00
            0000 0000 = State 2: 0x00

With patch (there is a bug in your bitmask i thinks...)

My Patch :
--- a/epan/dissectors/packet-ieee80211.c
+++ b/epan/dissectors/packet-ieee80211.c
@@ -13451,11 +13451,11 @@ add_tagged_field(packet_info *pinfo, proto_tree
*tree, tvbuff_t *tvb, int offset
         break;
       }
       offset += 2;
-      proto_tree_add_item(tree, hf_ieee80211_tag_type, tvb, offset, 1,
ENC_NA);
-      proto_tree_add_item(tree, hf_ieee80211_tag_tap1, tvb, offset, 2,
ENC_NA);
-      proto_tree_add_item(tree, hf_ieee80211_tag_state1, tvb, offset+1, 1,
ENC_NA);
-      proto_tree_add_item(tree, hf_ieee80211_tag_tap2, tvb, offset+2, 1,
ENC_NA);
-      proto_tree_add_item(tree, hf_ieee80211_tag_state2, tvb, offset+3, 1,
ENC_NA);
+      proto_tree_add_item(tree, hf_ieee80211_tag_type, tvb, offset, 4,
ENC_NA);
+      proto_tree_add_item(tree, hf_ieee80211_tag_tap1, tvb, offset, 4,
ENC_NA);
+      proto_tree_add_item(tree, hf_ieee80211_tag_state1, tvb, offset, 4,
ENC_NA);
+      proto_tree_add_item(tree, hf_ieee80211_tag_tap2, tvb, offset, 4,
ENC_NA);
+      proto_tree_add_item(tree, hf_ieee80211_tag_state2, tvb, offset, 4,
ENC_NA);
       offset += 4;
       break;
     }
@@ -17041,27 +17041,27 @@ proto_register_ieee80211 (void)

     {&hf_ieee80211_tag_type,
      {"Type", "wlan.sctor_id.type",
-      FT_UINT8, BASE_HEX, NULL, 0x0f,
+      FT_UINT32, BASE_HEX, NULL, 0x0f000000,
       NULL, HFILL }},

     {&hf_ieee80211_tag_tap1,
      {"Tap 1", "wlan.sctor_id.tap1",
-      FT_UINT16, BASE_HEX, NULL, 0x03f0,
+      FT_UINT32, BASE_HEX, NULL, 0x03f00000,
       NULL, HFILL }},

     {&hf_ieee80211_tag_state1,
      {"State 1", "wlan.sctor_id.state1",
-      FT_UINT8, BASE_HEX, NULL, 0xfc,
+      FT_UINT32, BASE_HEX, NULL, 0x00fc0000,
       NULL, HFILL }},

     {&hf_ieee80211_tag_tap2,
      {"Tap 2", "wlan.sctor_id.tap2",
-      FT_UINT8, BASE_HEX, NULL, 0xff,
+      FT_UINT32, BASE_HEX, NULL, 0x0000ff00,
       NULL, HFILL }},

     {&hf_ieee80211_tag_state2,
      {"State 2", "wlan.sctor_id.state2",
-      FT_UINT8, BASE_HEX, NULL, 0xff,
+      FT_UINT32, BASE_HEX, NULL, 0x000000ff,
       NULL, HFILL }},

     {&hf_ieee80211_tag_allocation_id,
-- 
It should be applied to BRP_Request, SSW, SSWF, DMG Capa (octet by octet if
needed), DMG Oper, Extended shcedule, sta availability, DMG_BEAM_REFINEMENT


Also always the typo about add_tag_relay_capableities

+proto_tree_add_text(tree, tvb, offset, 1, "AID of NextPCP %d: %d", i,
tvb_get_guint8(tvb, offset));
[...]
+      proto_tree_add_text(tree, tvb, offset, 1, "Remaining BI's: %d",
tvb_get_guint8(tvb, offset));
[...]
+      proto_tree_add_text(tree, tvb, offset, 2, "Request Token: %d",
tvb_get_letohs(tvb, offset));
Use proto_tree_add_item

Also why change the length of hf_ieee80211_fc_frame_type_subtype ? 

+  if (tag_len < 2) {
+    proto_tree_add_text (tree, tvb, *offset + 2, tag_len,
+      "Relay Capabilities: Error: Tag length must be 2 bytes long");
+    return;
Use expert_info

---

      You are receiving this mail because:

• You are watching all bug changes.