[libvirt] [PATCH] fix xenDaemonListDefinedDomains

[Jim Fehlig]

Matthias Bolte wrote:
> 2009/11/25 Jim Fehlig <jfehlig at novell.com>:
>   
>> Commit 790f0b3057787bb64da8c46c111ff8d3eff7b2af causes contents of names
>> array to be freed even on success, resulting in no listing of defined
>> but inactive Xen domains. Patch below fixes it.
>>
>> Regards,
>> Jim
>>
>>     
>
> Good catch, I just reviewed this commit to see if I've caused similar
> bugs elsewhere, but this seems to be the only one.
>   

I had looked elsewhere in that commit as well but didn't see any other
problems.

>   
>> Index: libvirt-0.7.4/src/xen/xend_internal.c
>> ===================================================================
>> --- libvirt-0.7.4.orig/src/xen/xend_internal.c
>> +++ libvirt-0.7.4/src/xen/xend_internal.c
>> @@ -4693,13 +4693,14 @@ xenDaemonListDefinedDomains(virConnectPt
>>          }
>>
>>          if (ret >= maxnames)
>> -            break;
>> +            goto out;
>>      }
>>
>>  error:
>>      for (i = 0; i < ret; ++i)
>>          VIR_FREE(names[i]);
>>
>> +out:
>>      sexpr_free(root);
>>      return(ret);
>>  }
>>     
>
> Your patch doesn't fix the problem in all situations. If maxnames is
> larger than the actual number of domains then goto out is never
> executed.
>   

Ah, yes - good catch.

> I also forgot to set ret to -1 after freeing the names, this needs to
> be fixed too.
>
> I propose the attached patch to solve this issues.
>
> Matthias
>   
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index d61e9e6..a0c7d77 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -4696,12 +4696,17 @@ xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, int maxnames
>              break;
>      }
>  
> +cleanup:
> +    sexpr_free(root);
> +    return(ret);
> +
>  error:
>      for (i = 0; i < ret; ++i)
>          VIR_FREE(names[i]);
>  
> -    sexpr_free(root);
> -    return(ret);
> +    ret = -1;
> +
> +    goto cleanup;
>  }
>  
>  /**
>   

ACK.  Thanks again!

Jim