[libvirt] Re: libvirt code review

[Steve Grubb]

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.

-Steve