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

Re: [libvirt] [PATCH] Strict check when listing domains



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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