[Gluster-devel] static analysis updated

[Kaleb S. KEITHLEY]

On 12/19/2016 01:06 PM, Jeff Darcy wrote:
>> Thank you Kaleb. Shall we start somwhere in terms of automation?
>>
>> The cppcheck results look the shortest[1]. If we can commit to fixing all of
>> them in the next 1 month, I can kick off a non-voting smoke job. We'll make
>> it
>> vote after 1 month. I guss the cli and experimental xlator (and a few more
>> xlators, please check log) owners need to confirm that this is something we
>> can
>> target to fix. And commit to keeping fixed.
> 
> Before we get to automation, shouldn't we have a discussion about what
> "defects" we should filter out?  For example, we already have a problem
> with compilers spitting out warnings about unused variables in generated
> code, and then promoting those warnings to errors.  Fixing those is more
> trouble than it's worth.  Static analyzers are going to produce even
> more reports of Things That Don't Really Matter, along with a few about
> Actual Serious Problems.  It's a characteristic of the genre.  If we
> don't make any explicit decisions about priorities, it will actually
> take us longer to fix all of the null-pointer errors and array overflows
> and memory leaks as people wade through a sea of lesser defects.
> 

At the moment we're only talking about cppcheck. cppcheck is medium
interesting because a) it's the least picky of all of them, and b) it's
what Ubuntu looks at and wants fixed before they'll ship it (versus our
PPA packages.)

Here's what we're looking at now on the master branch:
====================================================================
[cli/src/cli.c:504]: (error) va_list 'ap' used before va_start() was called.
[cli/src/cli.c:530]: (error) va_list 'ap' used before va_start() was called.
[contrib/libexecinfo/execinfo.c:359]: (error) Memory leak: rval
[extras/create_new_xlator/new-xlator-tmpl.c:13]: (error) syntax error
[extras/test/test-ffop.c:27]: (error) Buffer overrun possible for long
command line arguments.
[libglusterfs/src/logging.c:2315]: (error) va_list 'ap' used before
va_start() was called.
[tests/basic/fops-sanity.c:63]: (error) Buffer overrun possible for long
command line arguments.
[tests/bugs/replicate/bug-1250170-fsync.c:39]: (error) Memory leak: buffer
[xlators/experimental/fdl/src/dump-tmpl.c] ->
[xlators/experimental/fdl/src/dump-tmpl.c]: (error) syntax error
[xlators/experimental/fdl/src/recon-tmpl.c] ->
[xlators/experimental/fdl/src/recon-tmpl.c]: (error) syntax error
[xlators/experimental/jbr-client/src/fop-template.c] ->
[xlators/experimental/jbr-client/src/fop-template.c]: (error) syntax error
[xlators/experimental/jbr-server/src/all-templates.c] ->
[xlators/experimental/jbr-server/src/all-templates.c]: (error) syntax error
[xlators/features/changelog/lib/src/gf-history-changelog.c:803]: (error)
Null pointer dereference: this
[xlators/mount/fuse/src/fuse-helpers.c:253]: (error) Resource leak: fp
[xlators/storage/posix/src/posix-helpers.c:1097]: (error) Invalid number
of character '{' when these macros are defined: 'GF_DARWIN_HOST_OS'.
====================================================================

and since it wasn't clear in Nigel's mail, as a voting test, it would
only fail if someone causes an incremental warning over the status quo,
where the above is the current status quo.

Also it would be a non-voting test until we agree it's ready to be a
voting test. Translation: it's informational only, until we decide it's
ready for prime time, which might be never.

It does beg the question of how do we adjust the status quo upward if we
find something that's a false positive. cppcheck doesn't have many false
positives in my experience, so maybe it's a non-issue.

clang compile is also fairly forgiving, but clang analyze is too picky;
I don't expect to use it as a voting test. clang compile would also be
an "incremental" test.

coverity is a whole 'nuther ball of wax.

Waiting until we solve these problems just means we'll be waiting —
forever. IMO.

-- 

Kaleb