From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 03/10] libxl: fix return value of libxl__device_pci_destroy_all Date: Fri, 8 Apr 2016 10:19:13 +0200 Message-ID: <1460103553.13871.25.camel@citrix.com> References: <1459943163-18697-1-git-send-email-paulinaszubarczyk@gmail.com> <1459943163-18697-4-git-send-email-paulinaszubarczyk@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1134355647618428030==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aoRdX-0003TR-OO for xen-devel@lists.xenproject.org; Fri, 08 Apr 2016 08:19:27 +0000 In-Reply-To: <1459943163-18697-4-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 --===============1134355647618428030== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-6Q95B3DkF6ATPXDSHuK+" --=-6Q95B3DkF6ATPXDSHuK+ Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2016-04-06 at 13:45 +0200, Paulina Szubarczyk wrote: > Return rc value instead of always 0. >=20 I'd say "Propagate the error, insetad of always returning 0", but yeah, this is nitpicking! :-P Also, not my call, but I'd include this change inside of patch 1: both are simple enough that they can be made one, IMO (mentioning this new thing being done in the changelog of the resulting patch, of course). > --- a/tools/libxl/libxl_pci.c > +++ b/tools/libxl/libxl_pci.c > @@ -1602,7 +1602,7 @@ int libxl__device_pci_destroy_all(libxl__gc > *gc, uint32_t domid) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} > =C2=A0 While you're here, libxl conding style says that the 'int rc' variable used to host libxl error codes, and if necessary, to return them, should not be initialized, while this function does it. Again not my call, but I think you can get rid of the initialization too (and just assign 0 to it before the loop). Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-6Q95B3DkF6ATPXDSHuK+ 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 iEYEABECAAYFAlcHaYEACgkQk4XaBE3IOsRnyQCfeVgPjPENzsBTa9ssn/xGmWH5 U84AnjRUsvXWfo23UC/CcCDC2xpFNEwh =rD3J -----END PGP SIGNATURE----- --=-6Q95B3DkF6ATPXDSHuK+-- --===============1134355647618428030== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============1134355647618428030==--