Re: [PATCH] Strict check when listing domains

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

 



On 06/01/2010 11:55 AM, Eduardo Otubo wrote:
>> Sorry for the late review...
> 
> Thanks for the comments, all fixed :-)
> 
> diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
> index 8a9c7a6..423c95d 100644
> --- a/src/phyp/phyp_driver.c
> +++ b/src/phyp/phyp_driver.c

Would you mind sending the entire patch, including the git commit
message, rather than just the 'git diff' output?  That way, your name
will automatically show up in the commit message.  In this regards, 'git
send-email' can be quite a powerful tool, as it takes care of these details.

> -    if (nids == 0) {
> -        VIR_FREE(ids);
> -        return 0;
> -    }
> +    if (nids_numdomains != nids_listdomains){

Style: use ') {' (like what you just deleted) rather than '){'.

> +        VIR_ERROR(_("Unable to determine number of domains."));

s/ERROR/ERROR0/ since you didn't use % or extra args.

> +        goto err;
> +    }else if (nids_numdomains == 0 && nids_listdomains == 0)

more style: '} else'.  But here, the recent changes to HACKING recommend
that you either:

if (!cond)
    goto err;
else {
    ...
}

or:

if (cond) {
    ...
} else {
    goto err;
}

That is, if you have a one-line else clause, it should either be the
first clause encountered, or it should be in {} to match all the other
clauses.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]