Re: cppcheck on Dillo source

Hi Nick,

On Sat, Jul 09, 2016 at 12:35:10PM +0100, Nick Warne wrote:
> Hi all,
> 
> While waiting for the Tour De France to start on TV, I ran cppcheck on
> Dillo source for something to do - a few errors were picked up!
> 
> I have compacted the findings (with two comments from me)and attached it
> as a plain text file.

  Thanks for the report. I personally didn't know of cppcheck.

  We have used several code cleanup tools in the past and we usually
honour them as sometimes they catch important flaws.

  A bunch of patches was just committed. You may peruse them with
"hg log -vp" for the details/explanations.


> Checking ../../slackbuilds/dillo/hg/dillo/dpi/file.c...
> [../../slackbuilds/dillo/hg/dillo/dpi/file.c:356]: (error) syntax error
> 
>       if S_ISDIR (finfo->mode) {
> Nick->   maybe?:
>       if (S_ISDIR (finfo->mode)) {

  Done.

> Checking ../../slackbuilds/dillo/hg/dillo/dpid/dpid.c...
> [../../slackbuilds/dillo/hg/dillo/dpid/dpid.c:465]: (error) Resource leak: dpidrc_stream
> 
>       return -1;
> Nick->   maybe?:
>       return (-1);

  Done.

> Checking ../../slackbuilds/dillo/hg/dillo/dw/hyphenator.cc...
> [../../slackbuilds/dillo/hg/dillo/dw/hyphenator.cc:570]: (warning) %u in format
> string (no. 1) requires 'unsigned int *' but the argument type is 'int *'.
> Checking ../../slackbuilds/dillo/hg/dillo/dw/hyphenator.cc...
> [../../slackbuilds/dillo/hg/dillo/dw/hyphenator.cc:570]: (warning) %u in format
> string (no. 2) requires 'unsigned int *' but the argument type is 'int *'.

  It may be a trivial s/%u/%d/, but after reading it a bit I'd prefer
Sebastian to double check it.

  Not committed.

> 
> Checking ../../slackbuilds/dillo/hg/dillo/dw/iterator.cc...
> [../../slackbuilds/dillo/hg/dillo/dw/iterator.cc:124]: (error) Memory leak: eit3

  Done.

  AFAICS, this is a false positive.
  Patched to please cppcheck anyway.

> 
> Checking ../../slackbuilds/dillo/hg/dillo/dw/ooffloatsmgr.cc...
> [../../slackbuilds/dillo/hg/dillo/dw/ooffloatsmgr.cc:654]: (error) Possible null pointer dereference: vloat

  Well, the code should never reach there.
  Not changed.

  @Sebastian, BTW, coincidentally I'm using something like:

     SortedFloatsVector *list;
     list = isSubRefLeftFloat(ref) ? leftFloats : rightFloats;
     Float *vloat = list->get (getFloatIndexFromSubRef (ref));

in some patches I have, which also silence cppcheck. But maybe the
original construct was meant for adding more options (like absolute
positioned floats), but those may end in a third floats array, so I'm
not sure. Is it safe to use the above construct, or should I leave it
as is?


> Checking ../../slackbuilds/dillo/hg/dillo/lout/container.cc...
> [../../slackbuilds/dillo/hg/dillo/lout/container.cc:143]: (error) Common realloc mistake: 'array' nulled but not freed upon failure
> [../../slackbuilds/dillo/hg/dillo/lout/container.cc:177]: (error) Common realloc mistake: 'array' nulled but not freed upon failure

  Done.

> Checking ../../slackbuilds/dillo/hg/dillo/lout/misc.cc...
> [../../slackbuilds/dillo/hg/dillo/lout/misc.cc:172]: (error) Common realloc mistake: 'bits' nulled but not freed upon failure

  Done.

> Truck loads of:
> Checking ../../slackbuilds/dillo/hg/dillo/src/md5.c: ARCH_IS_BIG_ENDIAN...
> [../../slackbuilds/dillo/hg/dillo/src/md5.c:216]: (error) Uninitialized variable: X
> in md5.c

  Untouched: I'm not md5 savvy. Maybe corvid.

-- 
  Cheers
  Jorge.-