Wireshark · Wireshark-dev: Re: [Wireshark-dev] Lint of packet-tcp.c

["Sébastien Tandel" <sebastien@xxxxxxxxx>]

On Fri, Aug 15, 2008 at 11:57 AM, didier <dgautheron@xxxxxxxx> wrote:

Hi,
Le vendredi 15 août 2008 à 10:46 -0300, Sébastien Tandel a écrit :

>
> it is already not a so simple inspection path to check that if
> ipfd_head != NULL, it implies that msp != NULL ...  even for a
> human! :-p
> I don't think it costs a lot to have something like the following for
> the last "if" of the excerpt :
>
>
> if (msp && ipfd_head) {
>   ...
>   tcpinfo->seq = msp->seq;
>   ...
> }
>
>
> and it should let lint happy ... and humans too héhé ;)

And so if you use tools a little less brain dead than lint you get:
ipfd_head: always true condition.

It would be possible *if and only if* fragment_add always returns a non NULL element which is not the case. fragment_add returns non NULL /only/ when we have added all the fragments. ;)

My point is to say that I do prefer to have code easily understandable. And this case shows us - as per conversation here - that this code is not so obvious. :-/

Regards,

Sebastien

lint can find bugs, but it has a very high rate of false positive, you
have to live with it. I thought that after the debian debacle about ssl
we should be a little wiser about that kind of spurious code
modification.

Didier

_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@xxxxxxxxxxxxx
https://wireshark.org/mailman/listinfo/wireshark-dev