[05/18] vl.c: add deleted flag for deleting the handler.

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(-)

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.

Later, Juan.

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.
>