[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] fix xenDaemonListDefinedDomains



On Wed, Nov 25, 2009 at 01:54:05AM +0100, Matthias Bolte wrote:
> 2009/11/25 Jim Fehlig <jfehlig 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.
> 
> > 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.
> 
> 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, that looks right to me !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]