[libvirt] Re: libvirt code review

Daniel Veillard veillard at redhat.com
Tue Nov 10 16:19:20 UTC 2009


On Tue, Nov 10, 2009 at 11:07:31AM -0500, Steve Grubb wrote:
> On Tuesday 10 November 2009 07:00:36 am Daniel Veillard wrote:
> > > At line 2219 is an unusual if statement. Normally you do not see
> > > something  constructed as (!cpu)<0). That would seem to have meant cpu>=0
> > > which is more straightforward.
> > 
> >     if (def->cpumask &&
> >         !(cpus = virDomainCpuSetFormat(conn, def->cpumask,
> > def->cpumasklen)) < 0)       
> >         goto cleanup;
> > 
> >     But the function returns a string or NULL in case of error, I try to
> > fight again not fully parenthetized test expressions but I'm loosing
> > that fight unfortunately :-(
> > 
> >    in any case the < 0 test is bogus since it's a string
> > 
> >     if ((def->cpumask) &&
> >         ((cpus = virDomainCpuSetFormat(conn, def->cpumask,
> >          def->cpumasklen)) == NULL))
> >         goto cleanup
> > 
> > seems the right construct to me, as it tests if the call failed.
> 
> This looks a lot better. Thanks for looking into all these findings. I'll try 
> to do another sweep in a few months to see if anything new pops up.

  Okay, thanks, just tell me before, as the code tend to move fast,
I should be able to point your to a recent release or sugegst a
snapshot
   ftp://libvirt.org/libvirt/libvirt-git-snapshot.tar.gz

they are generated every hour or so :-)

   thanks again !

Daniel

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




More information about the libvir-list mailing list