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

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

On Fri, Aug 15, 2008 at 2:30 AM, Jaap Keuter <jaap.keuter@xxxxxxxxx> wrote:

Hi Chris,

Thanks for taking a look. I looked at your new log also, and still wonder how
lint get these 'Possible use of null pointer' results. Use of msp is guarded by
ipfd_head, so this should never cause a problem. I wonder what lint sees as path
to get in trouble.

desegment_tcp (... struct tcp_analysis *tcp)

{

  *msp = NULL;

  *ipfd_head = NULL;

  if (tcpd) {

    msp = se_tree_lookup();

  }

  if (msp) {

    ...

    ipfd_head = fragment_add();

    ...

  }

  ...

  if (ipfd_head) {

    ...

    tcpinfo->seq = msp->seq

  }

}

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

Furthermore, gcc already eliminates useless checks for NULL pointers ;)

 -fdelete-null-pointer-checks

           Use global dataflow analysis to identify and eliminate useless

           checks for null pointers.  The compiler assumes that dereferencing

           a null pointer would have halted the program.  If a pointer is

           checked after it has already been dereferenced, it cannot be null.

           In some environments, this assumption is not true, and programs can

           safely dereference null pointers.  Use

           -fno-delete-null-pointer-checks to disable this optimization for

           programs which depend on that behavior.

           Enabled at levels -O2, -O3, -Os.

Regards,

Sebastien

Thanx,
Jaap

Maynard, Chris wrote:

> The changes look good to me.  I re-ran lint on the updated file (with
> some of the warnings taken out just to make it a little bit easier to
> scan).  There are still a couple of "Possible use of null pointer"
> warnings that show up that seem to have merit and might also need some
> attention.  The first one is:
>                         tcpinfo->seq = msp->seq;
> epan\dissectors\packet-tcp.c(1615) : Warning 613: Possible use of null
> pointer
>     'msp' in left argument to operator '->' [Reference: file
>     epan\dissectors\packet-tcp.c: lines 1482, 1504]
>
> Thanks,
> Chris
>
>> -----Original Message-----
>> From: wireshark-dev-bounces@xxxxxxxxxxxxx [mailto:wireshark-dev-
>> bounces@xxxxxxxxxxxxx] On Behalf Of Jaap Keuter
>> Sent: Wednesday, August 13, 2008 5:50 PM
>> To: Developer support list for Wireshark
>> Subject: Re: [Wireshark-dev] Lint of packet-tcp.c
>>
>> Hi,
>>
>> I've committed revision 26010 with some fixes for the really obvious
>> cases.
>> Please review the changes, found here:
>> http://anonsvn.wireshark.org/viewvc/index.py?view=rev&revision=26010
>>
>> Most of the lint output is caused by:
>> - signed / unsigned differences
>> - TRY / CATCH / RETHROW macro
>> which could have merit, that is for further study.
>>
>> Thanx,
>> Jaap
>>
>>

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