On 15 July 2015 at 13:47, Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > On Wed, Jul 15, 2015 at 01:37:22PM +0100, Emil Velikov wrote: >> On 15 July 2015 at 12:47, Thierry Reding <thierry.reding@xxxxxxxxx> wrote: >> > On Tue, Jul 14, 2015 at 03:10:04PM +0100, Emil Velikov wrote: >> >> Spotted by looking for similar "let's assume fd == 0 is invalid" bugs. >> >> >> >> Cc: Thierry Reding <thierry.reding@xxxxxxxxx> >> >> Signed-off-by: Emil Velikov <emil.l.velikov@xxxxxxxxx> >> >> --- >> >> 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 = drmOpenWithType(NULL, BusID, type); >> >> - if (fd <= 0 || nr_fds == DRM_MAX_FDS) >> >> + if (fd < 0 || nr_fds == 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 ! >> >> 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 <treding@xxxxxxxxxx> > > irrespective of the above, given that they are two orthogonal issues. > Ack. Greatly appreciated. -Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
