From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 10/10] libxl: libxl_tmem functions improving coding style Date: Fri, 8 Apr 2016 10:54:37 +0200 Message-ID: <1460105677.13871.44.camel@citrix.com> References: <1459943163-18697-1-git-send-email-paulinaszubarczyk@gmail.com> <1459943163-18697-11-git-send-email-paulinaszubarczyk@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2686355123117515555==" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aoSC6-0006Vy-Oz for xen-devel@lists.xenproject.org; Fri, 08 Apr 2016 08:55:11 +0000 In-Reply-To: <1459943163-18697-11-git-send-email-paulinaszubarczyk@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Paulina Szubarczyk , 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 List-Id: xen-devel@lists.xenproject.org --===============2686355123117515555== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-OZfbAGWl0rRmOgkutd71" --=-OZfbAGWl0rRmOgkutd71 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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) > =C2=A0 > =C2=A0char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long) > =C2=A0{ > -=C2=A0=C2=A0=C2=A0=C2=A0int rc; > +=C2=A0=C2=A0=C2=A0=C2=A0int r; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0char _buf[32768]; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0GC_INIT(ctx); > =C2=A0 > -=C2=A0=C2=A0=C2=A0=C2=A0rc =3D xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_= TMEM_OP_LIST, > domid, 32768, use_long, > +=C2=A0=C2=A0=C2=A0=C2=A0r =3D xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_T= MEM_OP_LIST, > domid, 32768, use_long, > This is ok, but... > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0_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). > -=C2=A0=C2=A0=C2=A0=C2=A0if (rc < 0) { > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0LOGEV(ERROR, rc, "Can no= t get tmem list"); > +=C2=A0=C2=A0=C2=A0=C2=A0if (r < 0) { > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0LOGEV(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 --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-OZfbAGWl0rRmOgkutd71 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEABECAAYFAlcHcc0ACgkQk4XaBE3IOsSzOQCfQC2SXqBcjDvH+5JRsIj0scK8 U5MAn1WLTc+rCe/zlXRYFhvit8mmYW+1 =n2VR -----END PGP SIGNATURE----- --=-OZfbAGWl0rRmOgkutd71-- --===============2686355123117515555== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============2686355123117515555==--