-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Tomas Carnecky wrote: > This fixes a 'Result of operation is garbage or undefined' bug found > by clang. 'framebuffer.base' was read before being initialized.
If the problem is that framebuffer.base is not initialized, I think it would be better to just initialize it to NULL at the top. Having multiple goto labels opens the possibility for someone to add / modify code and use the wrong one. > Signed-off-by: Tomas Carnecky <t...@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. The order of these two operations shouldn't matter. > 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"); -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAksqfF0ACgkQX1gOwKyEAw8+/gCgh04mi2G0Y3R3ZGh+LTyP7EC+ W98AnRwTlwPxvRSTutekoCrk087qthjB =oO8i -----END PGP SIGNATURE----- _______________________________________________ xorg-devel mailing list xorg-devel@lists.x.org http://lists.x.org/mailman/listinfo/xorg-devel