Re: [Wireshark-commits] rev 47381: /trunk-1.8/epan/dissectors/ /trunk-1.8/epan/dissectors/: packet-tcp.c

["Maynard, Chris" <Christopher.Maynard () GTECH COM>]

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);

- Chris

-----Original Message-----
From: wireshark-dev-bounces () wireshark org [mailto:wireshark-dev-bounces () wireshark org] 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,  <eapache () wireshark org> 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 <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe