On Fri, Jul 27, 2012 at 6:14 PM, Pascal Quantin <[email protected]> wrote: > Hi all, > > while working on fixing a few Clang warnings in packet-smb-pipe.c, I > discovered that the hash_key variable in dissect_pipe_dcerpc() function is > never used. > According to the nice comment Guy put in revision 7777 above the variable > assignment, I have the feeling that it is actually useful in case of > reassembly in both directions and that the following patch should be > applied: > > Index: packet-smb-pipe.c > =================================================================== > --- packet-smb-pipe.c (révision 44082) > +++ packet-smb-pipe.c (copie de travail) > @@ -3348,7 +3348,7 @@ > * in this direction, by searching for its reassembly > * structure. > */ > - fd_head=fragment_get(pinfo, fid, dcerpc_fragment_table); > + fd_head=fragment_get(pinfo, hash_key, > dcerpc_fragment_table); > if(!fd_head){ > /* No reassembly, so this is a new pdu. check if the > dissector wants us to reassemble it or if we > @@ -3370,11 +3370,11 @@ > more data ? > */ > if(pinfo->desegment_len){ > - fragment_add_check(d_tvb, 0, pinfo, fid, > + fragment_add_check(d_tvb, 0, pinfo, > hash_key, > dcerpc_fragment_table, > dcerpc_reassembled_table, > 0, reported_len, TRUE); > - fragment_set_tot_len(pinfo, fid, > + fragment_set_tot_len(pinfo, hash_key, > dcerpc_fragment_table, > pinfo->desegment_len+reported_len); > } > @@ -3392,7 +3392,7 @@ > while(fd_head->next){ > fd_head=fd_head->next; > } > - fd_head=fragment_add_check(d_tvb, 0, pinfo, fid, > + fd_head=fragment_add_check(d_tvb, 0, pinfo, hash_key, > dcerpc_fragment_table, dcerpc_reassembled_table, > fd_head->offset+fd_head->len, > reported_len, TRUE); > @@ -3426,7 +3426,7 @@ > * up so that we don't have to distinguish between the first > * pass and subsequent passes? > */ > - fd_head=fragment_add_check(d_tvb, 0, pinfo, fid, > dcerpc_fragment_table, > + fd_head=fragment_add_check(d_tvb, 0, pinfo, hash_key, > dcerpc_fragment_table, > dcerpc_reassembled_table, 0, 0, TRUE); > if(!fd_head){ > /* we didnt find it, try any of the heuristic dissectors > > Am I right or should we remove the hash_key variable? > > Regards, > Pascal.
I'm not sure what the correct course of action is, but there's already a bug for it. I found the same issue with CppCheck a few weeks ago :) https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7451 Cheers, Evan ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <[email protected]> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
