| Message ID | 66ae190f-827e-0f5d-b4d9-1692dea5507e@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 08/03/2016 05:57 PM, John Ferlan wrote: > > > On 08/03/2016 05:37 PM, Cole Robinson wrote: >> On 08/03/2016 09:40 AM, John Ferlan wrote: >>> >>> >>> On 07/28/2016 08:02 AM, Jovanka Gulicoska wrote: >>>> Add node device event handling infrastructure to node_device_event.[ch] >>>> --- >>>> src/Makefile.am | 5 + >>>> src/conf/node_device_event.c | 234 +++++++++++++++++++++++++++++++++++++++++++ >>>> src/conf/node_device_event.h | 59 +++++++++++ >>>> src/libvirt_private.syms | 5 + >>>> 4 files changed, 303 insertions(+) >>>> create mode 100644 src/conf/node_device_event.c >>>> create mode 100644 src/conf/node_device_event.h >>>> >>> >>> [...] >>> >>>> diff --git a/src/conf/node_device_event.c b/src/conf/node_device_event.c >>>> new file mode 100644 >>>> index 0000000..61bc912 >>> >>> [...] >>> >>>> + >>>> +/** >>>> + * virNodeDeviceEventLifecycleNew: >>>> + * @name: name of the node device object the event describes >>>> + * @type: type of lifecycle event >>>> + * @detail: more details about @type >>>> + * >>>> + * Create a new node device lifecycle event. >>>> + */ >>>> +virObjectEventPtr >>>> +virNodeDeviceEventLifecycleNew(const char *name, >>>> + int type, >>>> + int detail) >>>> +{ >>>> + virNodeDeviceEventLifecyclePtr event; >>>> + >>>> + if (virNodeDeviceEventsInitialize() < 0) >>>> + return NULL; >>>> + >>>> + if (!(event = virObjectEventNew(virNodeDeviceEventLifecycleClass, >>>> + virNodeDeviceEventDispatchDefaultFunc, >>>> + VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE, >>>> + 0, name, NULL, name))) >>> ^^^^ >>> >>> This has caused a Coverity build failure since the prototype has: >>> >>> ATTRIBUTE_NONNULL(6) >>> >> >> I think just dropping it is fine? The code was updated to handle uuid=NULL >> >> diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h >> index 92c25d4..27b461f 100644 >> --- a/src/conf/object_event_private.h >> +++ b/src/conf/object_event_private.h >> @@ -106,6 +106,6 @@ virObjectEventNew(virClassPtr klass, >> const unsigned char *uuid, >> const char *key) >> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) >> - ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7); >> + ATTRIBUTE_NONNULL(7); >> >> #endif >> >>> It gets even worse in the function and needs to be resolved before the >>> "next" release. >>> >> >> I can't parse this sentence... are there additional issues? >> > virObjectEventNew(...,const unsigned char *uuid,...) > ... > memcpy(event->meta.uuid, uuid, VIR_UUID_BUFLEN); > ... > > e.g. > memcpy(event->meta.uuid, NULL, VIR_UUID_BUFLEN); > But the full code from virObjectEventNew handles that case by checking for non-NULL uuid first: if (uuid) memcpy(event->meta.uuid, uuid, VIR_UUID_BUFLEN); - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 08/03/2016 06:39 PM, John Ferlan wrote: > > > On 08/03/2016 06:30 PM, Cole Robinson wrote: >> On 08/03/2016 05:57 PM, John Ferlan wrote: >>> >>> >>> On 08/03/2016 05:37 PM, Cole Robinson wrote: >>>> On 08/03/2016 09:40 AM, John Ferlan wrote: >>>>> >>>>> >>>>> On 07/28/2016 08:02 AM, Jovanka Gulicoska wrote: >>>>>> Add node device event handling infrastructure to node_device_event.[ch] >>>>>> --- >>>>>> src/Makefile.am | 5 + >>>>>> src/conf/node_device_event.c | 234 +++++++++++++++++++++++++++++++++++++++++++ >>>>>> src/conf/node_device_event.h | 59 +++++++++++ >>>>>> src/libvirt_private.syms | 5 + >>>>>> 4 files changed, 303 insertions(+) >>>>>> create mode 100644 src/conf/node_device_event.c >>>>>> create mode 100644 src/conf/node_device_event.h >>>>>> >>>>> >>>>> [...] >>>>> >>>>>> diff --git a/src/conf/node_device_event.c b/src/conf/node_device_event.c >>>>>> new file mode 100644 >>>>>> index 0000000..61bc912 >>>>> >>>>> [...] >>>>> >>>>>> + >>>>>> +/** >>>>>> + * virNodeDeviceEventLifecycleNew: >>>>>> + * @name: name of the node device object the event describes >>>>>> + * @type: type of lifecycle event >>>>>> + * @detail: more details about @type >>>>>> + * >>>>>> + * Create a new node device lifecycle event. >>>>>> + */ >>>>>> +virObjectEventPtr >>>>>> +virNodeDeviceEventLifecycleNew(const char *name, >>>>>> + int type, >>>>>> + int detail) >>>>>> +{ >>>>>> + virNodeDeviceEventLifecyclePtr event; >>>>>> + >>>>>> + if (virNodeDeviceEventsInitialize() < 0) >>>>>> + return NULL; >>>>>> + >>>>>> + if (!(event = virObjectEventNew(virNodeDeviceEventLifecycleClass, >>>>>> + virNodeDeviceEventDispatchDefaultFunc, >>>>>> + VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE, >>>>>> + 0, name, NULL, name))) >>>>> ^^^^ >>>>> >>>>> This has caused a Coverity build failure since the prototype has: >>>>> >>>>> ATTRIBUTE_NONNULL(6) >>>>> >>>> >>>> I think just dropping it is fine? The code was updated to handle uuid=NULL >>>> >>>> diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h >>>> index 92c25d4..27b461f 100644 >>>> --- a/src/conf/object_event_private.h >>>> +++ b/src/conf/object_event_private.h >>>> @@ -106,6 +106,6 @@ virObjectEventNew(virClassPtr klass, >>>> const unsigned char *uuid, >>>> const char *key) >>>> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) >>>> - ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7); >>>> + ATTRIBUTE_NONNULL(7); >>>> >>>> #endif >>>> >>>>> It gets even worse in the function and needs to be resolved before the >>>>> "next" release. >>>>> >>>> >>>> I can't parse this sentence... are there additional issues? >>>> >>> virObjectEventNew(...,const unsigned char *uuid,...) >>> ... >>> memcpy(event->meta.uuid, uuid, VIR_UUID_BUFLEN); >>> ... >>> >>> e.g. >>> memcpy(event->meta.uuid, NULL, VIR_UUID_BUFLEN); >>> >> >> But the full code from virObjectEventNew handles that case by checking for >> non-NULL uuid first: >> >> if (uuid) >> memcpy(event->meta.uuid, uuid, VIR_UUID_BUFLEN); >> > > Hmm.... strange, my coverity environment AFU'd.... Ahhh I see why. > > OK, removing the ATTRIBUTE_NONNULL(6) works just fine > > ACK for that > > John > > Long story short - recent update to f24 has for some reason caused > compilation failures for "-Wnonnull-compare"'s within the coverity build > environment, so I made some local changes to get past it while I figured > out what the issue was. That was one of the many changes. > Got it. I pushed that change: commit e584615b81b5cabb252b1866171bde25b4f06d05 Author: Cole Robinson <crobinso@redhat.com> Date: Wed Aug 3 18:45:50 2016 -0400 conf: events: Fix coverity warning Since 2bfa75134 virObjectEventNew can be passed a NULL 'uuid' value, so drop the ATTRIBUTE_NONNULL annotation Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/src/conf/object_event_private.h b/src/conf/object_event_private.h index 92c25d4..27b461f 100644 --- a/src/conf/object_event_private.h +++ b/src/conf/object_event_private.h @@ -106,6 +106,6 @@ virObjectEventNew(virClassPtr klass, const unsigned char *uuid, const char *key) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) - ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7); + ATTRIBUTE_NONNULL(7); #endif