Re: [Qemu-devel] [PATCH] rng: switch request queue to QSIMPLEQ

[Paolo Bonzini]

On 04/03/2016 10:19, Ladi Prosek wrote:
> On Fri, Mar 4, 2016 at 10:12 AM, Paolo Bonzini <address@hidden> wrote:
>>
>>
>> On 04/03/2016 09:04, Ladi Prosek wrote:
>>>>>>> +    QSIMPLEQ_INIT(&s->requests);
>>>>>>>  }
>>>>>
>>>>> This init here isn't necessary, the accessors for the queue will take
>>>>> care of this.
>>> We are basically purging the queue here and we want to leave it in a
>>> consistent state. Without the QSIMPLEQ_INIT the queue head would
>>> become a pair of dangling pointers. Let me know if I misunderstood
>>> your comment.
>>
>> It wouldn't, check out QSIMPLEQ_REMOVE_HEAD:
>>
>> #define QSIMPLEQ_REMOVE_HEAD(head, field) do {
>>     if (((head)->sqh_first = (head)->sqh_first->field.sqe_next) == NULL)
>>         (head)->sqh_last = &(head)->sqh_first;
>> } while (/*CONSTCOND*/0)
>>
>> The queue would become { NULL, &s->requests.sqh_first }.  So the
>> QSIMPLEQ_INIT is indeed redundant.
> 
> Right, but we're not running QSIMPLEQ_REMOVE_HEAD in this function. We
> iterate the queue and free all elements without writing anything to
> the head or to the next ptr. This is the only "write" we do in
> rng_backend_free_requests.

Ah, sorry, I was convinced that rng_backend_free_request did the remove,
but now I remember checking it yesterday (after making the same
reasoning as Amit) and indeed it doesn't. :)

So the patch is okay.  It's just a slightly unusual use of
QSIMPLEQ_FOREACH_SAFE.

Paolo