From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44242) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCin5-0002yF-0P for qemu-devel@nongnu.org; Wed, 08 Jul 2015 02:25:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZCin1-0002Bd-Oc for qemu-devel@nongnu.org; Wed, 08 Jul 2015 02:25:06 -0400 Date: Wed, 8 Jul 2015 08:24:56 +0200 From: Thomas Huth Message-ID: <20150708082456.15e927fa@thh440s> In-Reply-To: <20150708043029.GL17857@voom.redhat.com> References: <1436148670-6592-1-git-send-email-aik@ozlabs.ru> <1436148670-6592-14-git-send-email-aik@ozlabs.ru> <20150707092311.728e2cd7@thh440s> <559BA465.50009@ozlabs.ru> <20150707122125.0e58b09e@thh440s> <559BB25E.8000008@ozlabs.ru> <20150708043029.GL17857@voom.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/KKatcP+rDNzzdLWR9u7POa8"; protocol="application/pgp-signature" Subject: Re: [Qemu-devel] [PATCH qemu v10 13/14] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Michael Roth , Alexey Kardashevskiy , qemu-devel@nongnu.org, Gavin Shan , Alex Williamson , qemu-ppc@nongnu.org --Sig_/KKatcP+rDNzzdLWR9u7POa8 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Wed, 8 Jul 2015 14:30:29 +1000 David Gibson wrote: > On Tue, Jul 07, 2015 at 09:05:02PM +1000, Alexey Kardashevskiy wrote: > > On 07/07/2015 08:21 PM, Thomas Huth wrote: > > >On Tue, 7 Jul 2015 20:05:25 +1000 > > >Alexey Kardashevskiy wrote: > > > > > >>On 07/07/2015 05:23 PM, Thomas Huth wrote: > > >>>On Mon, 6 Jul 2015 12:11:09 +1000 > > >>>Alexey Kardashevskiy wrote: ... > > >>>>@@ -698,14 +768,18 @@ static int vfio_connect_container(VFIOGroup *= group, AddressSpace *as) > > >>>> > > >>>> container->iommu_data.type1.initialized =3D true; > > >>>> > > >>>>- } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMM= U)) { > > >>>>+ } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMM= U) || > > >>>>+ ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_I= OMMU)) { > > >>>>+ bool v2 =3D !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_T= CE_v2_IOMMU); > > >>> > > >>>That "!!" sounds somewhat wrong here. I think you either want to che= ck > > >>>for "ioctl() =3D=3D 1" (because only in this case you can be sure th= at v2 > > >>>is supported), or you can simply omit the "!!" because you're 100% s= ure > > >>>that the ioctl only returns 0 or 1 (and never a negative error code). > > >> > > >> > > >>The host kernel does not return an error on these ioctls, it returns = 0 or > > >>1. And "!!" is shorter than "(bool)". VFIO_CHECK_EXTENSION for Type1 = does > > >>exactly the same already. > > > > > >Simply using nothing instead is even shorter than using "!!". The > > >compiler is smart enough to convert from 0 and 1 to bool. > > >"!!" is IMHO quite ugly and should only be used when it is really > > >necessary. > >=20 > >=20 > > imho it is not but either way I'd rather follow the existing style, > > especially if I do literally the same thing (checking IOMMU version). U= nless > > the original author tells me to convert all the existing occurences of = "!!" > > to "!=3D0" (or something like this) before I post new ones. > >=20 > > Alex, should I get rid of "!!"s in the patch? >=20 > I think !! is the lesser evil here. The trouble is that in C "bool" > is not a first-class datatype, but just a typedef for some integer > type. Which means that, confusingly, (bool)2 !=3D (bool)1. So using > the !! trick to force a value to be either 0 or 1 when assigning it to > a bool variable is probably a good idea. That was maybe the case > 15 years ago, but since C99, there is a proper bool type in C, as far as I know. But I am also not an expert here... However, I tried the following small test program: #include #include int main() { bool a =3D 1; bool b =3D 2; printf("a=3D%i b=3D%i\n", a, b); return 0; } ... and indeed, it prints out "a=3D1 b=3D1" here, so the "2" got properly changed to "true" :-) Anyway, that was already too much bike-shed painting now, if you want to keep the "!!", then keep it, that's fine for me, too. Thomas --Sig_/KKatcP+rDNzzdLWR9u7POa8 Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJVnMI4AAoJEC7Z13T+cC21KZ4QAIAMO8+s8iks4yq8m2uMMJwD eRcDg16j2ASOOf2hXIQ/VdhQb0Tgv9PVuD0qwLjH0MHb6rgn4a68KsuV0gnvEUU9 l5kmnOAcNSOYzfh+EY/ilexaHrqfdw2r0cUl38Pl/pm40uY/bPEKUSn3x2joqhwR ncmkUaRVd9T/ODxVtwGpxFbZ2n2vY2q0htZ6LeybN/jKwtYwtP3B+RutK8QuUSMo gvFqjIQnaw4fqoZt1jX5vIHwOhESi8heizJYZzEouxDPhhHRzdg6SZb7bMBYVu8A Ph9i+U8k9iHroqM3p6dYaNNDXOEagabXghrSlLpjq8iZbh9Jip+GcFr//deoPTOZ urTUnYpnt4m7ySpZC4nzlKxEg5lR/dEiDZD8PuXnuxjv37cjFcxP8oEpdDayU+Tk xvbV4THlQ1kW0jsvx7Ne2c/PaR0Fu1NIL9DetXkHZOjOVRoz/12q0mowsif+ySlP HN4KmXLsMuIPHrB00h9WuXVD0/u1/iOaBaQVB4l8HKQs2dqCbPn/Z9R6nYQM8dMZ 93JBHd4J+mh5GtpH6j8WLKZd6/T7bpdyiOFaGJL5cS3HFzQzoU5Mee4Bl93OOeAs 9E8JdM98c53A+/U6ReLr29BT6/AsdGlq2heYJNJqPzBDLo5pLjc+LSJozKatMfVg ppbkdPOtxytPKetoso82 =nIrY -----END PGP SIGNATURE----- --Sig_/KKatcP+rDNzzdLWR9u7POa8--