On 05/22/2015 10:31 AM, Jiri Denemark wrote:
> On Fri, May 22, 2015 at 10:16:26 -0400, John Ferlan wrote:
>>
>>
>> On 05/21/2015 06:42 PM, Jiri Denemark wrote:
>>> Our usage of pthread conditions does not allow a single thread to wait
>>> for several events from different sources. This is because the condition
>>> is bound to the source of the event. We can invert the usage by giving
>>> each thread its own condition and providing APIs for registering this
>>> thread condition with several sources. Each of the sources can then
>>> signal the thread condition.
>>>
>>> Thread queues also support several threads to be registered with a
>>> single event source, which can either wakeup all waiting threads or just
>>> the first one.
>>>
>>> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
>>> ---
>>> po/POTFILES.in | 1 +
>>> src/Makefile.am | 2 +
>>> src/libvirt_private.syms | 15 ++
>>> src/util/virthreadqueue.c | 343 ++++++++++++++++++++++++++++++++++++++++++++++
>>> src/util/virthreadqueue.h | 54 ++++++++
>>> 5 files changed, 415 insertions(+)
>>> create mode 100644 src/util/virthreadqueue.c
>>> create mode 100644 src/util/virthreadqueue.h
>>>
>>
>> Ran the series through Coverity and came back with this gem
>>
>>> +
>>> +void
>>> +virThreadQueueFree(virThreadQueuePtr queue)
>>> +{
>>> + if (!queue)
>>> + return;
>>> +
>>> + while (queue->head)
>>> + virThreadQueueRemove(queue, queue->head);
>>
>> (3) Event cond_true: Condition "queue->head", taking true branch
>> (6) Event loop_begin: Jumped back to beginning of loop
>> (7) Event cond_true: Condition "queue->head", taking true branch
>>
>> 283 while (queue->head)
>>
>> (4) Event freed_arg: "virThreadQueueRemove" frees "queue->head". [details]
>> (5) Event loop: Jumping back to the beginning of the loop
>> (8) Event deref_arg: Calling "virThreadQueueRemove" dereferences freed pointer "queue->head". [details]
>>
>> 284 virThreadQueueRemove(queue, queue->head);
>
> False positive. If virThreadQueueRemove frees queue->head, it also
> updates it with queue->head->next.
>
I understand and looked at the code, but I think this is a case where
pass by reference and pass by value makes a difference. Upon return
what causes queue->head to be evaluated again? The call passes
queue->head by value and removes it from queue. Upon return nothing
causes queue->head to be evaluated again thus wouldn't we enter the loop
the second time with the same address?
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list