On Thu, Jan 31, 2013 at 11:39 AM, Maynard, Chris <[email protected]> wrote: > Thanks for back-porting the fix to 1.8. My apologies for not doing so before. > > I don't think fixes to *all* Coverity issues necessarily need to be > back-ported though, fixes such as > http://anonsvn.wireshark.org/viewvc?view=revision&revision=47073, for > example. But your point is valid, and in general yes, it's a good idea. I > will try to be more diligent with scheduling Coverity fixes to be backported. > > As for the duplicate check, that does look strange. To me, it would make > sense to remove the second one. I'm surprised that Coverity (or Clang or VS > Code Analysis) didn't flag the second block as "Logically Dead Code". > > Maybe it would also make sense to move the 1st check *before* this line: > next_tvb = tvb_new_subset_remaining(tvb, offset);
Agreed, that makes sense to me. Evan > - Chris > > -----Original Message----- > From: [email protected] > [mailto:[email protected]] On Behalf Of Evan Huus > Sent: Wednesday, January 30, 2013 7:45 PM > To: Wireshark Developer List > Subject: Re: [Wireshark-dev] [Wireshark-commits] rev 47381: > /trunk-1.8/epan/dissectors/ /trunk-1.8/epan/dissectors/: packet-tcp.c > > Two points of interest here: > > - The original fix in trunk was a coverity fix and wasn't backported at the > time (I assume) because it wasn't known to fix an actual crash. > Should we have some sort of policy to avoid this, by e.g. backporting fixes > for all coverity issues when possible? > > - The exact check being made happens in two different places in trunk with > *exactly* the same code. Is that intentional (in which case there should be > an explanatory comment) or can one of them be removed? > > Cheers, > Evan > > On Wed, Jan 30, 2013 at 7:41 PM, <[email protected]> wrote: >> http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=47381 >> >> User: eapache >> Date: 2013/01/30 04:41 PM >> >> Log: >> Manually rediscover r43185 to fix >> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8274 >> >> Directory: /trunk-1.8/epan/dissectors/ >> Changes Path Action >> +1 -1 packet-tcp.c Modified >> > > -- > > CONFIDENTIALITY NOTICE: The information contained in this email message is > intended only for use of the intended recipient. If the reader of this > message is not the intended recipient, you are hereby notified that any > dissemination, distribution or copying of this communication is strictly > prohibited. If you have received this communication in error, please > immediately delete it from your system and notify the sender by replying to > this email. Thank you. > ___________________________________________________________________________ > 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 ___________________________________________________________________________ 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
