[Qemu-devel] Re: [PATCH 05/18] vl.c: add deleted flag for deleting the handler.

[Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>]

Juan Quintela wrote:
> Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>  wrote:
>> Make deleting handlers robust against deletion of any elements in a
>> handler by using a deleted flag like in file descriptors.
>>
>> Signed-off-by: Yoshiaki Tamura<tamura.yoshiaki@lab.ntt.co.jp>
>> ---
>>   vl.c |   13 +++++++++----
>>   1 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index b436952..4e263c3 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1158,6 +1158,7 @@ static void nographic_update(void *opaque)
>>   struct vm_change_state_entry {
>>       VMChangeStateHandler *cb;
>>       void *opaque;
>> +    int deleted;
>>       QLIST_ENTRY (vm_change_state_entry) entries;
>>   };
>>
>> @@ -1178,8 +1179,7 @@ VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>>
>>   void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
>>   {
>> -    QLIST_REMOVE (e, entries);
>> -    qemu_free (e);
>> +    e->deleted = 1;
>>   }
>>
>>   void vm_state_notify(int running, int reason)
>> @@ -1188,8 +1188,13 @@ void vm_state_notify(int running, int reason)
>>
>>       trace_vm_state_notify(running, reason);
>>
>> -    for (e = vm_change_state_head.lh_first; e; e = e->entries.le_next) {
>> -        e->cb(e->opaque, running, reason);
>
> this needs to become:
>
>> +    QLIST_FOREACH(e,&vm_change_state_head, entries) {
>> +        if (e->deleted) {
>> +            QLIST_REMOVE(e, entries);
>> +            qemu_free(e);
>> +        } else {
>> +            e->cb(e->opaque, running, reason);
>> +        }
>
>     VMChangeState_entry *next;
>
>     QLIST_FOREACH_SAFE(e,&vm_change_state_head, entries, next) {
>        .....
>
>    Otherwise you are accessing "e" after qemu_free and being put out of
>    the list.

You're right.  Thanks.

Yoshi

>
> Later, Juan.
>