[PATCH] [glx] Use goto-style error handling in __glXDRIscreenProbe()

Tomas Carnecky tom at dbservice.com
Sun Dec 6 10:40:06 PST 2009


This fixes a 'Result of operation is garbage or undefined' bug found
by clang. 'framebuffer.base' was read before being initialized.

Signed-off-by: Tomas Carnecky <tom at dbservice.com>
---

This patch slightly reorders the cleanup. In the original code DRICloseConnection()
would go before dlclose(screen->driver). But since we open the dri connection before
dlopen'ing the driver, I thought it would make sense to dlclose the driver before 
closing the dri connection. I don't know if that can be a problem, so someone
familiar with the code please review.

 glx/glxdri.c |   45 +++++++++++++++++++++++----------------------
 1 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/glx/glxdri.c b/glx/glxdri.c
index 6122653..cbbf240 100644
--- a/glx/glxdri.c
+++ b/glx/glxdri.c
@@ -996,7 +996,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
 
     if (!DRIOpenConnection(pScreen, &hSAREA, &BusID)) {
 	LogMessage(X_ERROR, "AIGLX error: DRIOpenConnection failed\n");
-	goto handle_error;
+	goto err_free;
     }
 
     fd = drmOpenOnce(NULL, BusID, &newlyopened);
@@ -1004,12 +1004,12 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
     if (fd < 0) {
 	LogMessage(X_ERROR, "AIGLX error: drmOpenOnce failed (%s)\n",
 		   strerror(-fd));
-	goto handle_error;
+	goto err_close_dri;
     }
 
     if (drmGetMagic(fd, &magic)) {
 	LogMessage(X_ERROR, "AIGLX error: drmGetMagic failed\n");
-	goto handle_error;
+	goto err_close_drm;
     }
 
     version = drmGetVersion(fd);
@@ -1027,7 +1027,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
 
     if (newlyopened && !DRIAuthConnection(pScreen, magic)) {
 	LogMessage(X_ERROR, "AIGLX error: DRIAuthConnection failed\n");
-	goto handle_error;
+	goto err_close_drm;
     }
 
     /* Get device name (like "tdfx") and the ddx version numbers.
@@ -1039,7 +1039,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
 				&ddx_version.patch,
 				&driverName)) {
 	LogMessage(X_ERROR, "AIGLX error: DRIGetClientDriverName failed\n");
-	goto handle_error;
+	goto err_close_drm;
     }
 
     snprintf(filename, sizeof filename, "%s/%s_dri.so",
@@ -1049,14 +1049,14 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
     if (screen->driver == NULL) {
 	LogMessage(X_ERROR, "AIGLX error: dlopen of %s failed (%s)\n",
 		   filename, dlerror());
-        goto handle_error;
+        goto err_close_drm;
     }
 
     extensions = dlsym(screen->driver, __DRI_DRIVER_EXTENSIONS);
     if (extensions == NULL) {
 	LogMessage(X_ERROR, "AIGLX error: %s exports no extensions (%s)\n",
 		   driverName, dlerror());
-	goto handle_error;
+	goto err_dlclose;
     }
     
     for (i = 0; extensions[i]; i++) {
@@ -1075,7 +1075,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
 	LogMessage(X_ERROR,
 		   "AIGLX error: %s does not export required DRI extension\n",
 		   driverName);
-	goto handle_error;
+	goto err_dlclose;
     }
 
     /*
@@ -1088,7 +1088,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
 			  &framebuffer.size, &framebuffer.stride,
 			  &framebuffer.dev_priv_size, &framebuffer.dev_priv)) {
 	LogMessage(X_ERROR, "AIGLX error: XF86DRIGetDeviceInfo failed\n");
-	goto handle_error;
+	goto err_dlclose;
     }
 
     framebuffer.width = pScreen->width;
@@ -1100,7 +1100,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
     if (status != 0) {
 	LogMessage(X_ERROR, "AIGLX error: drmMap of framebuffer failed (%s)\n",
 		   strerror(-status));
-	goto handle_error;
+	goto err_dlclose;
     }
 
     /* Map the SAREA region.  Further mmap regions may be setup in
@@ -1110,7 +1110,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
     if (status != 0) {
 	LogMessage(X_ERROR, "AIGLX error: drmMap of SAREA failed (%s)\n",
 		   strerror(-status));
-	goto handle_error;
+	goto err_unmap_fb;
     }
     
     screen->driScreen =
@@ -1128,7 +1128,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
     if (screen->driScreen == NULL) {
 	LogMessage(X_ERROR,
 		   "AIGLX error: Calling driver entry point failed\n");
-	goto handle_error;
+	goto err_unmap_sarea;
     }
 
     screen->base.fbconfigs = glxConvertConfigs(screen->core, driConfigs);
@@ -1167,21 +1167,22 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
 
     return &screen->base;
 
- handle_error:
-    if (pSAREA != NULL)
-	drmUnmap(pSAREA, SAREA_MAX);
+err_unmap_sarea:
+    drmUnmap(pSAREA, SAREA_MAX);
 
-    if (framebuffer.base != NULL)
-	drmUnmap((drmAddress)framebuffer.base, framebuffer.size);
+err_unmap_fb:
+    drmUnmap((drmAddress)framebuffer.base, framebuffer.size);
 
-    if (fd >= 0)
-	drmCloseOnce(fd);
+err_dlclose:
+    dlclose(screen->driver);
 
-    DRICloseConnection(pScreen);
+err_close_drm:
+    drmCloseOnce(fd);
 
-    if (screen->driver)
-        dlclose(screen->driver);
+err_close_dri:
+    DRICloseConnection(pScreen);
 
+err_free:
     xfree(screen);
 
     LogMessage(X_ERROR, "AIGLX: reverting to software rendering\n");
-- 
1.6.5.4




More information about the xorg-devel mailing list