Re: [PATCH v2 10/10] libxl: libxl_tmem functions improving coding style

From: Dario Faggioli <dario.faggioli@citrix.com>
To: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>,
	xen-devel@lists.xenproject.org, roger.pau@citrix.com,
	George.Dunlap@eu.citrix.com
Cc: ian.jackson@eu.citrix.com, wei.liu2@citrix.com, ian.campbell@citrix.com
Subject: Re: [PATCH v2 10/10] libxl: libxl_tmem functions improving coding style
Date: Fri, 8 Apr 2016 10:54:37 +0200	[thread overview]
Message-ID: <1460105677.13871.44.camel@citrix.com> (raw)
In-Reply-To: <1459943163-18697-11-git-send-email-paulinaszubarczyk@gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1631 bytes --]

On Wed, 2016-04-06 at 13:46 +0200, Paulina Szubarczyk wrote:
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -6238,14 +6238,14 @@ uint32_t libxl_vm_get_start_time(libxl_ctx
> *ctx, uint32_t domid)
>  
>  char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)
>  {
> -    int rc;
> +    int r;
>      char _buf[32768];
>      GC_INIT(ctx);
>  
> -    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST,
> domid, 32768, use_long,
> +    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST,
> domid, 32768, use_long,
>
This is ok, but...
>                           _buf);
>
...this now have the wrong indentation: it should be aligned with the
opening '(' in the line above (so, basically, you have to kill one
white space and move it back by one column).

> -    if (rc < 0) {
> -        LOGEV(ERROR, rc, "Can not get tmem list");
> +    if (r < 0) {
> +        LOGEV(ERROR, r, "Can not get tmem list");
>
libxc functions are supposed to, on failure, set errno and alwas return
-1. Using LOGEV and passing r to it means that you're always logging -1
as error code.

I think 'LOG(ERROR, "blabla")' is what should be used here.

And these same comments apply to all the other hunks of the patch.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel