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

From: "Sébastien Tandel" <sebastien@xxxxxxxxx>
Date: Fri, 15 Aug 2008 10:46:36 -0300


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