2012/7/28 Evan Huus <[email protected]> > 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 >
Hi Evan, thanks for the reminder: I missed this bug ;) I guess Guy would be the best able to answer this question (if he remembers the code he wrote in 2003!). Regards, Pascal.
___________________________________________________________________________ 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
