Re: [libvirt] [PATCH] Fix build with GCC's static analysis

On 03/27/2017 06:46 AM, Martin Kletzander wrote: > STREQ_NULLABLE returns true if both parameters are NULL. And that's > not what we want here. We just want to skop comparing source nodes stop

> that don't have that info set. The function wouldn't make much sense > with nodeName == NULL, so we don't need to check that. Moreover, the > function's declaration uses ATTRIBUDE_NONNULL for nodeName, which not ATTRIBUTE

> only means that function expects the parameter not to be NULL, but > actually tells the compiler that it can optimize out the NULL checks. > That way it could end up calling strcmp on NULL (either nodeformat or > nodebacking). GCC figures this out if libvirt is compiled with > lv_cv_static_analysis=yes, unfortunately not everyone uses that. > > Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; > --- > src/util/virstoragefile.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > ACK

John > diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c > index 3bcb69bf6206..0ac707962102 100644 > --- a/src/util/virstoragefile.c > +++ b/src/util/virstoragefile.c > @@ -3809,8 +3809,8 @@ virStorageSourceFindByNodeName(virStorageSourcePtr top, > *index = 0; > > for (tmp = top; tmp; tmp = tmp->backingStore) { > - if (STREQ_NULLABLE(tmp->nodeformat, nodeName) || > - STREQ_NULLABLE(tmp->nodebacking, nodeName)) > + if ((tmp->nodeformat && STREQ(tmp->nodeformat, nodeName)) || > + (tmp->nodebacking && STREQ(tmp->nodebacking, nodeName))) > return tmp; > > if (index) > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list