[libvirt PATCH 0/3] fix ATTRIBUTE_NONNULL issues

Pavel Hrdina posted 3 patches 4 years, 1 month ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1583928792.git.phrdina@redhat.com
src/conf/domain_conf.h | 2 +-
src/libvirt_internal.h | 3 +--
src/util/vircommand.h  | 5 ++---
3 files changed, 4 insertions(+), 6 deletions(-)
[libvirt PATCH 0/3] fix ATTRIBUTE_NONNULL issues
Posted by Pavel Hrdina 4 years, 1 month ago
Pavel Hrdina (3):
  domain_conf: fix ATTRIBUTE_NONNULL for virDomainDefFormat
  libvirt_internal: fix ATTRIBUTE_NONNULL for virStateInitialize
  vircommand: fix ATTRIBUTE_NONNULL for virCommandAddArg*

 src/conf/domain_conf.h | 2 +-
 src/libvirt_internal.h | 3 +--
 src/util/vircommand.h  | 5 ++---
 3 files changed, 4 insertions(+), 6 deletions(-)

-- 
2.24.1

Re: [libvirt PATCH 0/3] fix ATTRIBUTE_NONNULL issues
Posted by Ján Tomko 4 years, 1 month ago
Missing blurb in the cover letter.

On a Wednesday in 2020, Pavel Hrdina wrote:
>Pavel Hrdina (3):
>  domain_conf: fix ATTRIBUTE_NONNULL for virDomainDefFormat
>  libvirt_internal: fix ATTRIBUTE_NONNULL for virStateInitialize
>  vircommand: fix ATTRIBUTE_NONNULL for virCommandAddArg*
>

Do these actually help Coverity do its job or all they're good for
is finding out where we failed to update the attributes?

Jano

> src/conf/domain_conf.h | 2 +-
> src/libvirt_internal.h | 3 +--
> src/util/vircommand.h  | 5 ++---
> 3 files changed, 4 insertions(+), 6 deletions(-)
>
>-- 
>2.24.1
>
Re: [libvirt PATCH 0/3] fix ATTRIBUTE_NONNULL issues
Posted by Pavel Hrdina 4 years, 1 month ago
On Wed, Mar 11, 2020 at 01:25:50PM +0100, Ján Tomko wrote:
> Missing blurb in the cover letter.
> 
> On a Wednesday in 2020, Pavel Hrdina wrote:
> > Pavel Hrdina (3):
> >  domain_conf: fix ATTRIBUTE_NONNULL for virDomainDefFormat
> >  libvirt_internal: fix ATTRIBUTE_NONNULL for virStateInitialize
> >  vircommand: fix ATTRIBUTE_NONNULL for virCommandAddArg*
> > 
> 
> Do these actually help Coverity do its job or all they're good for
> is finding out where we failed to update the attributes?

Now that I'm trying to setup coverity scans using scan.coverity.com they
actually help coverity do its job and I managed to run into these issues
while building libvirt using coverity.

Pavel
Re: [libvirt PATCH 0/3] fix ATTRIBUTE_NONNULL issues
Posted by John Ferlan 4 years, 1 month ago

On 3/11/20 8:34 AM, Pavel Hrdina wrote:
> On Wed, Mar 11, 2020 at 01:25:50PM +0100, Ján Tomko wrote:
>> Missing blurb in the cover letter.
>>
>> On a Wednesday in 2020, Pavel Hrdina wrote:
>>> Pavel Hrdina (3):
>>>  domain_conf: fix ATTRIBUTE_NONNULL for virDomainDefFormat
>>>  libvirt_internal: fix ATTRIBUTE_NONNULL for virStateInitialize
>>>  vircommand: fix ATTRIBUTE_NONNULL for virCommandAddArg*
>>>
>>
>> Do these actually help Coverity do its job or all they're good for
>> is finding out where we failed to update the attributes?
> 
> Now that I'm trying to setup coverity scans using scan.coverity.com they
> actually help coverity do its job and I managed to run into these issues
> while building libvirt using coverity.
> 
> Pavel
> 

Good luck with Coverity issues - if you'd like my list of work-around
hacks I don't mind sending you the list of 55 or so local patches in my
build environment. There's even more with more recent coverity releases,
but I haven't taken the time to work through them.

These types of build issues can also be seen by enabling STATIC_ANALYSIS
for a build - IOW: outside of coverity...

FWIW: I've tried this for vircommand.h a while ago, see:

https://www.redhat.com/archives/libvir-list/2018-December/msg00548.html

The only other one I had in my backlog of hacks around things that in
general the community hasn't accepted historically is for virnetdevtap.h
and the virNetDevTapReattachBridge prototype for argument 2 because in
the source code the @brname is used in a STREQ_NULLABLE thus it also
gets flagged.

John

Re: [libvirt PATCH 0/3] fix ATTRIBUTE_NONNULL issues
Posted by Pavel Hrdina 4 years, 1 month ago
On Wed, Mar 11, 2020 at 09:23:50AM -0400, John Ferlan wrote:
> 
> 
> On 3/11/20 8:34 AM, Pavel Hrdina wrote:
> > On Wed, Mar 11, 2020 at 01:25:50PM +0100, Ján Tomko wrote:
> >> Missing blurb in the cover letter.
> >>
> >> On a Wednesday in 2020, Pavel Hrdina wrote:
> >>> Pavel Hrdina (3):
> >>>  domain_conf: fix ATTRIBUTE_NONNULL for virDomainDefFormat
> >>>  libvirt_internal: fix ATTRIBUTE_NONNULL for virStateInitialize
> >>>  vircommand: fix ATTRIBUTE_NONNULL for virCommandAddArg*
> >>>
> >>
> >> Do these actually help Coverity do its job or all they're good for
> >> is finding out where we failed to update the attributes?
> > 
> > Now that I'm trying to setup coverity scans using scan.coverity.com they
> > actually help coverity do its job and I managed to run into these issues
> > while building libvirt using coverity.
> > 
> > Pavel
> > 
> 
> Good luck with Coverity issues - if you'd like my list of work-around
> hacks I don't mind sending you the list of 55 or so local patches in my
> build environment. There's even more with more recent coverity releases,
> but I haven't taken the time to work through them.

Thanks, I know that it will probably take some time to process all the
issues.  Most of them are false positive, however, the tool I'm planning
to use, scan.coverity.com, has a possibility to provide "modeling file"
where you can write simple C-code like functions to help coverity with
the false positives.

That would be nice to see the list of patches that you already have,
maybe they could be somehow used to create the modeling file.

> These types of build issues can also be seen by enabling STATIC_ANALYSIS
> for a build - IOW: outside of coverity...
> 
> FWIW: I've tried this for vircommand.h a while ago, see:
> 
> https://www.redhat.com/archives/libvir-list/2018-December/msg00548.html
> 
> The only other one I had in my backlog of hacks around things that in
> general the community hasn't accepted historically is for virnetdevtap.h
> and the virNetDevTapReattachBridge prototype for argument 2 because in
> the source code the @brname is used in a STREQ_NULLABLE thus it also
> gets flagged.

In this specific case it's not even coverity issue but build issue with
STATIC_ANALYSIS enabled.  There are two possible solutions, removing the
ATTRIBUTE_NONNULL or disabling nonnull-compare gcc check for broken gcc.
I'm OK with both solutions.

I'm not planning to introduce any hacks into our code-base except for
things like sa_assert() which we already use.

Thanks,

Pavel