From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH libdrm 3/4] xf86drm: fix incorrect fd comparison in drmOpenOnce{,WithType} Date: Wed, 15 Jul 2015 14:47:21 +0200 Message-ID: <20150715124720.GF15045@ulmo.nvidia.com> References: <1436883005-6163-1-git-send-email-emil.l.velikov@gmail.com> <1436883005-6163-3-git-send-email-emil.l.velikov@gmail.com> <20150715114723.GC15045@ulmo.nvidia.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1488481675==" Return-path: Received: from mail-pa0-f48.google.com (mail-pa0-f48.google.com [209.85.220.48]) by gabe.freedesktop.org (Postfix) with ESMTPS id 84CE96E3CA for ; Wed, 15 Jul 2015 05:47:30 -0700 (PDT) Received: by padck2 with SMTP id ck2so23706532pad.0 for ; Wed, 15 Jul 2015 05:47:30 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Emil Velikov Cc: ML dri-devel List-Id: dri-devel@lists.freedesktop.org --===============1488481675== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="qftxBdZWiueWNAVY" Content-Disposition: inline --qftxBdZWiueWNAVY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 15, 2015 at 01:37:22PM +0100, Emil Velikov wrote: > On 15 July 2015 at 12:47, Thierry Reding wrote: > > On Tue, Jul 14, 2015 at 03:10:04PM +0100, Emil Velikov wrote: > >> Spotted by looking for similar "let's assume fd =3D=3D 0 is invalid" b= ugs. > >> > >> Cc: Thierry Reding > >> Signed-off-by: Emil Velikov > >> --- > >> xf86drm.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/xf86drm.c b/xf86drm.c > >> index 2c17d11..39c6e2d 100644 > >> --- a/xf86drm.c > >> +++ b/xf86drm.c > >> @@ -2619,7 +2619,7 @@ int drmOpenOnceWithType(const char *BusID, int *= newlyopened, int type) > >> } > >> > >> fd =3D drmOpenWithType(NULL, BusID, type); > >> - if (fd <=3D 0 || nr_fds =3D=3D DRM_MAX_FDS) > >> + if (fd < 0 || nr_fds =3D=3D DRM_MAX_FDS) > > > > Consider what happens if we have DRM_MAX_FDS file descriptors open and > > the call to drmOpenWithType() succeeds. We'll end up returning the file > > descriptor as is, but we won't keep track. > > > > I suppose this could have been on purpose, so that the device could be > > opened even if the file descriptor couldn't be cached anymore. One > > potential problem with that could be that the open-once restriction > > would be silently ignored. That may not be desirable. > > > Thanks for reviewing ! >=20 > Yes I have considered the issue. It's slightly different bug (fixed by > using dynamic allocation?) than what this patch aims at. This fix came > along for consistency sake rather than me caring about this legacy > API. Right, I had meant to say: Reviewed-by: Thierry Reding irrespective of the above, given that they are two orthogonal issues. > Can you please follow up if you're interested ? I don't think I have any interest in this API, just occurred to me that it wasn't well documented and the behaviour seemed at odds with what documentation there is (i.e. the function name). Thierry --qftxBdZWiueWNAVY Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJVplZVAAoJEN0jrNd/PrOhbz8QAKxNFyoqiwBlnJhjdhrL6dhC Bfhqb0N94fV5g070xV17UhRrBxkouDrasBG6WtJ38qmhZ6auUss9wmaMDbMwNkgX bvDbzLQ0Qoa0ALl90DFsfGP8xs90HHS/8KXEOnsPL6Ik8+U2YXCnbe3jnh24LGka SKL9P5NmnPMGoo4hx2nNyPbz3UTP//vav143mrUVLbLsXebp1z0VgceS0bdWrXTb eqEhquBeThTucQscw3YCNFHs5NNUqgZtWgBvNjokU0B+USmpn/t2mY0e63ay0n4U 1ukDZLUdpqOoZHaGWHmidk6qtrxKb623BdRZ9v7M1LzCWrrByDTfPzFMDRd4TZI/ NQg2DW0P0dwED+Z+5kJakmgQjufCC5g375G2Luw/9ADnfErO0KwClg0378WBrWcl I3VWiXBV/No4HI3ewUq/4Y0XzIMvGV1plbyJQ4jPTFOS4zYFFsF3M1Ea9Tr6QXs0 INZ1h8roFSSFsmR4+jOF5xLvuALFHjII7XqzAuzcoI91Hh0TBXHPgOXy6LZyvBrg 3DnfKqgpfTy6x4jk2oPfB1LIQchkQQeZ86NQol4iluxI8lOhrw3/z3lddg77ni9u VxgjRFWXvPLCrA56/b2Duf+zHPaDadTqG3J0yO9qYUFYO35r0nIB/riI+lvF1eGH W9H8iLVfIOnzjXvOEp5g =olHi -----END PGP SIGNATURE----- --qftxBdZWiueWNAVY-- --===============1488481675== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1488481675==--