Re: [libvirt] [PATCH v2 2/8] conf: add node_device_event handling

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);