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

[Cole Robinson]

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