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

[Cole Robinson]

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 at 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