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.-