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@redhat.com> --- 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