xf86-video-intel: src/intel_device.c src/intel_module.c

[Chris Wilson]

 src/intel_device.c |   58 +++++++++++++++++++++++++++++++++++++++++++++++------
 src/intel_module.c |   56 ++-------------------------------------------------
 2 files changed, 55 insertions(+), 59 deletions(-)

New commits:
commit 282507af8e932e49b31d5f20958a61c137df590d
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Tue Jul 2 09:34:57 2013 +0100

    intel: Move the validation of the KMS device into the open routine
    
    Currently we leak the fd should we open the device node and decide that
    is not a GEM/KMS kernel driver. The simplest way to perform the cleanup
    upon failure is to move the checking for GEM/KMS into the device open
    routine.
    
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/intel_device.c b/src/intel_device.c
index da4d21e..c5f0a38 100644
--- a/src/intel_device.c
+++ b/src/intel_device.c
@@ -38,6 +38,7 @@
 #include <xf86drm.h>
 #include <xf86drmMode.h>
 #include <xf86_OSproc.h>
+#include <i915_drm.h>
 
 #include "intel_driver.h"
 
@@ -63,6 +64,45 @@ static inline void intel_set_device(ScrnInfoPtr scrn, struct intel_device *dev)
 	xf86GetEntityPrivate(scrn->entityList[0], intel_device_key)->ptr = dev;
 }
 
+static Bool is_i915_device(int fd)
+{
+	drm_version_t version;
+	char name[5] = "";
+
+	memset(&version, 0, sizeof(version));
+	version.name_len = 4;
+	version.name = name;
+
+	if (drmIoctl(fd, DRM_IOCTL_VERSION, &version))
+		return FALSE;
+
+	return strcmp("i915", name) == 0;
+}
+
+static int __intel_check_device(int fd)
+{
+	int ret;
+
+	/* Confirm that this is a i915.ko device with GEM/KMS enabled */
+	ret = is_i915_device(fd);
+	if (ret) {
+		struct drm_i915_getparam gp;
+		gp.param = I915_PARAM_HAS_GEM;
+		gp.value = &ret;
+		if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
+			ret = FALSE;
+	}
+	if (ret) {
+		struct drm_mode_card_res res;
+
+		memset(&res, 0, sizeof(res));
+		if (drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res))
+			ret = FALSE;
+	}
+
+	return ret;
+}
+
 static int fd_set_cloexec(int fd)
 {
 	int flags;
@@ -145,14 +185,14 @@ int intel_open_device(int entity_num,
 
 	fd = __intel_open_device(pci, &local_path);
 	if (fd == -1)
-		return -1;
+		goto err_path;
+
+	if (!__intel_check_device(fd))
+		goto err_close;
 
 	dev = malloc(sizeof(*dev));
-	if (dev == NULL) {
-		free(local_path);
-		close(fd);
-		return -1;
-	}
+	if (dev == NULL)
+		goto err_close;
 
 	dev->path = local_path;
 	dev->fd = fd;
@@ -162,6 +202,12 @@ int intel_open_device(int entity_num,
 	xf86GetEntityPrivate(entity_num, intel_device_key)->ptr = dev;
 
 	return fd;
+
+err_close:
+	close(fd);
+err_path:
+	free(local_path);
+	return -1;
 }
 
 int intel_get_device(ScrnInfoPtr scrn)
diff --git a/src/intel_module.c b/src/intel_module.c
index 0183111..b4d4f1c 100644
--- a/src/intel_module.c
+++ b/src/intel_module.c
@@ -28,12 +28,7 @@
 #include "config.h"
 #endif
 
-#include <unistd.h>
 #include <xf86Parser.h>
-#include <xf86drm.h>
-#include <xf86drmMode.h>
-#include <i915_drm.h>
-
 #include <xorgVersion.h>
 
 #if XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,6,99,0,0)
@@ -434,51 +429,6 @@ static Bool intel_driver_func(ScrnInfoPtr pScrn,
 	}
 }
 
-static Bool is_i915_device(int fd)
-{
-	drm_version_t version;
-	char name[5] = "";
-
-	memset(&version, 0, sizeof(version));
-	version.name_len = 4;
-	version.name = name;
-
-	if (drmIoctl(fd, DRM_IOCTL_VERSION, &version))
-		return FALSE;
-
-	return strcmp("i915", name) == 0;
-}
-
-static Bool has_kernel_mode_setting(int entity_num,
-				    const struct pci_device *dev,
-				    const char *path)
-{
-	int ret, fd;
-
-	fd = intel_open_device(entity_num, dev, path);
-	if (fd == -1)
-		return FALSE;
-
-	/* Confirm that this is a i915.ko device with GEM/KMS enabled */
-	ret = is_i915_device(fd);
-	if (ret) {
-		struct drm_i915_getparam gp;
-		gp.param = I915_PARAM_HAS_GEM;
-		gp.value = &ret;
-		if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
-			ret = FALSE;
-	}
-	if (ret) {
-		struct drm_mode_card_res res;
-
-		memset(&res, 0, sizeof(res));
-		if (drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res))
-			ret = FALSE;
-	}
-
-	return ret;
-}
-
 #if !UMS_ONLY
 extern XF86ConfigPtr xf86configptr;
 
@@ -574,7 +524,7 @@ static Bool intel_pci_probe(DriverPtr		driver,
 			    struct pci_device	*device,
 			    intptr_t		match_data)
 {
-	if (!has_kernel_mode_setting(entity_num, device, NULL)) {
+	if (intel_open_device(entity_num, device, NULL) == -1) {
 #if KMS_ONLY
 		return FALSE;
 #else
@@ -605,8 +555,8 @@ intel_platform_probe(DriverPtr driver,
 	if (!dev->pdev)
 		return FALSE;
 
-	if (!has_kernel_mode_setting(entity_num, dev->pdev,
-				     xf86_get_platform_device_attrib(dev, ODEV_ATTRIB_PATH)))
+	if (intel_open_device(entity_num, dev->pdev,
+			      xf86_get_platform_device_attrib(dev, ODEV_ATTRIB_PATH)) == -1)
 		return FALSE;
 
 	/* Allow ourselves to act as a slaved output if not primary */