On Fri, May 22, 2015 at 13:48:16 -0400, John Ferlan wrote:
>
>
> 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?
This would be a serious bug in the compiler. The function also gets the
queue pointer, which means the compiler should not consider queue->head
was unchanged.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list