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

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.