improvements in RELOAD dissection

This issue was migrated from bug 6317 in our old bug tracker.

Original bug information:

Reporter: Stéphane Bryant
Status: RESOLVED FIXED
Product: Wireshark
Component: GTK+ UI
OS: All
Platform: Other
Version: 1.7.x (Experimental)

Attachments:

0001-Improvement-in-reload-dissection.patch: patch
0001-packet-reload.c-removed-unused-variables.patch: remove the variables that the previous patch left unused

Stéphane Bryant said:

Created attachment 6945 patch

Build Information: wireshark 1.7.0

Copyright 1998-2011 Gerald Combs <gerald@‍wireshark.org> and contributors. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Compiled (64-bit) with GTK+ 2.24.4, with Cairo 1.10.2, with Pango 1.28.4, with GLib 2.28.6, with libpcap 1.1.1, with libz 1.2.3.4, with POSIX capabilities (Linux), with threads support, with SMI 0.4.8, with c-ares 1.7.3, with Lua 5.1, with Python 2.7.1+, with GnuTLS 2.8.6, with Gcrypt 1.4.6, with MIT Kerberos, with GeoIP, with PortAudio V19-devel (built Mar 18 2011 15:49:34), without AirPcap.

Running on Linux 2.6.38-10-generic, with locale en_US.UTF-8, with libpcap version 1.1.1, with libz 1.2.3.4, GnuTLS 2.8.6, Gcrypt 1.4.6.

Built using gcc 4.5.2.

- Follow closely draft-ietf-p2psip-base-18
- Added support for draft-ietf-p2psip-base-18,
  draft-ietf-p2psip-sip-06,
  draft-ietf-p2psip-service-discovery-03,
  draft-ietf-p2psip-self-tuning-04,
  draft-ietf-p2psip-diagnostics-06,
  draft-zong-p2psip-drr-00,
- Handoff to the xml dissectors for
  configuration data
- export the message content dissection function
  in the new packet-reload.h file for use in
  related protocols (draft-hautakorpi-p2psip-with-hip-01)

This patch has been fuzz-tested

added enhancement uigtk versiondev labels

Bill Meier said:

I'll take a look at the patch.

(I can deal with the fact the patch won't apply cleanly that because of the recent proto_tree_add_item() encoding-parameter changes that I've made).

Bill Meier said:

Patch (updated with "encoding parameter changes") committed in SVN #39301

Thanks !

From me:
- Fix a few C++ style comments;
- Fix Windows compile warnings:
packet-reload.c(2156) ... conversion from 'guint64' to 'guint32', possible loss of data
packet-reload.c(3528) ... conversion from 'guint64' to 'guint32', possible loss of data

Note: Additional fix yet req'd since checkhf.pl gives:
  ERROR: NO ARRAY: packet-reload.c, hf_reload_dmflag_underlay_hop
  Unused entry: packet-reload.c, hf_reload_storeddata_signature
  Unused entry: packet-reload.c, hf_reload_storeddataspecifiers

(Compile is OK).

I'll leave this bug open until the "hf" issues are fixed.

Bill Meier said:

Additional (minor) compile warnings to be fixed:

packet-reload.c: In function 'dissect_datavalue':
packet-reload.c:1792:12: warning: variable 'exists' set but not used [-Wunused-but-set-variable]

packet-reload.c: In function 'dissect_storeddata':
packet-reload.c:2148:17: warning: variable 'ti_storagetime' set but not used [-Wunused-but-set-variable]


packet-reload.c: In function 'dissect_reload_message':
packet-reload.c:3903:15: warning: variable 'msg_method_str_short' set but not used [-Wunused-but-set-variable]
packet-reload.c:3902:15: warning: variable 'msg_class_str_short' set but not used [-Wunused-but-set-variable]

Stéphane Bryant said:

Created attachment 7179 remove the variables that the previous patch left unused

Bill Meier said:

Committed in SVN #39308.

Thanks

Re:
  ERROR: NO ARRAY: packet-reload.c, hf_reload_dmflag_underlay_hop

I've added an entry to hf[]:

    { &hf_reload_dmflag_underlay_hop,
      { "UNDERLAY_HOP", "reload.dmflags.underlay_hop", FT_BOOLEAN, 1, TFS(&tfs_set_notset), 0x0,
        NULL, HFILL
      }
    },

If this is incorrect or the dmflag_underlay stuff should be removed altogether, please let me know.

Christopher Maynard said:

Coverity reports in CID 1307 an UNREACHABLE bug at line 2877, which is at the end of dissect_extensiveroutingmodeoption():

Event unreachable: This code cannot be reached: "return local_offset;".
2877 	  return local_offset;


By the way, tools/checkfiltername.pl (provided by Michael Mann) reported the following display filter naming errors, which I've corrected in r39393 (along with a couple of overflow errors that Coverity found and reported in CID's 1305 & 1306):

epan/dissectors/packet-reload.c (3 (of 274) fields)
5358 reload.reload.redirserviceprovider.data.namespace duplicates PROTOABBREV of reload
5363 reload.reload.redirserviceprovider.data.level duplicates PROTOABBREV of reload
5368 reload.reload.redirserviceprovider.data.node duplicates PROTOABBREV of reload

Christopher Maynard said:

(In reply to comment #6)  
> Coverity reports in CID 1307 an UNREACHABLE bug at line 2877, which is at the  
> end of dissect_extensiveroutingmodeoption():  
>  
> Event unreachable: This code cannot be reached: "return local_offset;".  
> 2877       return local_offset;  

I don't see CID 1307 anymore so someone must have fixed this.  Closing the bug.

closed