Re: [libvirt] [PATCH 01/10] util: Introduce thread queues

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(a)redhat.com&gt; >> --- >> 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.