From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH] libxl: fix handling returns in libxl_get_version_info() Date: Fri, 12 Feb 2016 12:00:06 +0100 Message-ID: <1455274806.3148.347.camel@citrix.com> References: <1455179459-3392-1-git-send-email-write.harmandeep@gmail.com> <1455184090.3148.275.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4709173279258819632==" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aUBSh-0008Te-05 for xen-devel@lists.xenproject.org; Fri, 12 Feb 2016 11:00:31 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Harmandeep Kaur Cc: xen-devel@lists.xenproject.org, wei.liu2@citrix.com, ian.jackson@eu.citrix.com, Ian Campbell , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org --===============4709173279258819632== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-MvSlztRLg3Bn+7WohKnt" --=-MvSlztRLg3Bn+7WohKnt Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2016-02-12 at 16:22 +0530, Harmandeep Kaur wrote: > On Thu, Feb 11, 2016 at 3:18 PM, Dario Faggioli > wrote: > >=C2=A0 > > Another thing that you should check, and probably quickly mention > > too, > > is whether or not the callers of these functions --the ones inside > > xen.git of course-- are prepared to deal with this. I mean, > > returning > > NULL on failure of xc_version() is, IMO, the right thing to do here > > anyway, but it is something important to know whether more work is > > needed to fix the callers as well, or if we're just fine. > >=20 > > To do so, search (e.g., with grep or cscope) for uses of > > libxl_get_version_info inside the tools/ dir of xen.git. For > > instance, > > there is one such use in xl_cmdimpl.c, in the output_xeninfo() > > function, which looks like it would interact well with this > > patch... > > Can you confirm this is the case also for all the other instances > > and > > note it down here? >=20 > I think NULL may not be handled properly in auto_autoballoon() in > tools/libxl/xl.c Other instances seems okay to me. >=20 So, here: =C2=A0 =C2=A0 info =3D libxl_get_version_info(ctx); =C2=A0=C2=A0=C2=A0=C2=A0if (!info) =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return 1; /* default to on = */ Why do you think this is a problem? It looks like this call site is actually prepared to see and handle a NULL return value... > Should I fix the caller, too? >=20 If there are issues with them, yes. If there aren't just put something like "xxx callers are already fine xxx" in the changelog. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-MvSlztRLg3Bn+7WohKnt 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 v1 iEYEABECAAYFAla9uzYACgkQk4XaBE3IOsTFOACfb0Hb2xStDCuKbM+MApzlzw+n AgYAnj76PO9zOko59okjRsxcxAV+uIpL =2RGA -----END PGP SIGNATURE----- --=-MvSlztRLg3Bn+7WohKnt-- --===============4709173279258819632== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============4709173279258819632==--