From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 2/5] libxl: make GC_FREE reachable in libxl_get_scheduler() Date: Tue, 5 Jan 2016 09:49:29 +0100 Message-ID: <1451983769.27063.43.camel@citrix.com> References: <1450091323.16856.24.camel@citrix.com> <1451279808-25264-1-git-send-email-jtotto@uwaterloo.ca> <1451279808-25264-3-git-send-email-jtotto@uwaterloo.ca> <1451924973.13361.183.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3123658515797164134==" Return-path: In-Reply-To: <1451924973.13361.183.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Joshua Otto , xen-devel@lists.xen.org Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com, george.dunlap@eu.citrix.com, ian.jackson@eu.citrix.com, czylin@uwaterloo.ca, hjarmstr@uwaterloo.ca List-Id: xen-devel@lists.xenproject.org --===============3123658515797164134== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-cTXNZi4kvIMjAhtRXzpg" --=-cTXNZi4kvIMjAhtRXzpg Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2016-01-04 at 16:29 +0000, Ian Campbell wrote: > On Mon, 2015-12-28 at 00:16 -0500, Joshua Otto wrote: > > Coverity CID 1343309 > >=20 > > This patch preserves the multiple error paths in order to avoid > > meaninglessly assigning the ERROR_FAIL libxl error code to the > > return variable, which is of type libxl_scheduler. >=20 > Which makes me think that the existing code is bogus to return an > error > code as a libxl_scheduler too, since that is not very different to > the > bogus assignment. >=20 Indeed. > Given that a caller really ought to be handling > LIBXL_SCHEDULER_UNKNOWN as > a return value, even if it is also written to expect negative error > values > as well, so I reckon we can get away with changing the return to > SCHEDULER_UNKNOWN in the error case. Either the caller already > handles > that, or it was already buggy in not doing so. >=20 Again, FWIW, I think this indeed is the proper way forward. About callers, xl is, of course, quite easy to change. I just quickly checked libvirt, and I think things will just continue to work there too. In fact, libxl_get_scheduler() is used 3 times in there, of which: =C2=A0- 2 of them, explicitly check for the result to=C2=A0 =C2=A0 =C2=A0be=C2=A0LIBXL_SCHEDULER_CREDIT, and errors if it is not (as Cr= edit1 is =C2=A0 =C2=A0the only supported scheduler in libvirt for now) =C2=A0- 1 explicitly check for the result to be either _SEDF, _CREDIT,=C2= =A0 =C2=A0 =C2=A0_CREDIT2 or _ARINC653, and errors out if it's something else[1= ] For other users, I agree that they should be handling or start to handle LIBXL_SCHEDULER_UNKNOWN. *Maybe*, just as a "mitigation" measure", we can redefine the enum and make "unknown"=3D-1... or is something like that to be considered an API change as well? I know, it looks really an hack, and it would remain wrong, for a caller, to check for a libxl_scheduler object to be equal to ERROR_FAIL, but maybe it's worth at least being considered. Thanks and Regards, Dario [1] note to self, send a patch to update that (e.g., adding RTDS and removing SEDF, when adequate, depending on Xen version) --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-cTXNZi4kvIMjAhtRXzpg 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 iEYEABECAAYFAlaLg5kACgkQk4XaBE3IOsQjuwCfedkuiIXHTsAwwIZamiJo+b4z WUsAoKRabwPAxL1Ag1UwI6EGVCQ/oAcF =UvV8 -----END PGP SIGNATURE----- --=-cTXNZi4kvIMjAhtRXzpg-- --===============3123658515797164134== 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 --===============3123658515797164134==--