GLX: Change __glx_Main to use the libGLX-allocated __GLXapiImports table.

__glx_Main now takes a pointer to the __GLXapiImports struct and fills it in.

The __GLXapiImports struct is now allocated and zeroed by libGLX. This makes it
possible to add an optional element to the end of the struct without breaking
backward compatibility.

5 files changed, 87 insertions, 62 deletions

diff --git a/include/glvnd/GLdispatchABI.h b/include/glvnd/GLdispatchABI.h
index 5329b49..16afb3d 100644
--- a/include/glvnd/GLdispatchABI.h
+++ b/include/glvnd/GLdispatchABI.h
@@ -158,8 +158,8 @@ typedef struct __GLdispatchPatchCallbacksRec {
                                DispatchPatchLookupStubOffset lookupStubOffset);
 
     /*!
-     * Called by libglvnd to notify the current vendor that it no longer owns
-     * the top-level entrypoints.
+     * (OPTIONAL) Called by libglvnd to notify the current vendor that it no
+     * longer owns the top-level entrypoints.
      *
      * Libglvnd will take care of the restoring the entrypoints back to their
      * original state. The vendor library must not try to modify them.
diff --git a/include/glvnd/libglxabi.h b/include/glvnd/libglxabi.h
index 63d6186..52ff3b9 100644
--- a/include/glvnd/libglxabi.h
+++ b/include/glvnd/libglxabi.h
@@ -300,25 +300,30 @@ typedef struct __GLXapiImportsRec {
     /*!
      * (OPTIONAL) Callbacks by which the vendor library may re-write libglvnd's
      * entrypoints at make current time, provided no other contexts are current
-     * and the TLS model supports this functionality.  This is a performance
+     * and the TLS model supports this functionality. This is a performance
      * optimization that may not be available at runtime; the vendor library
-     * must not depend on this functionality for correctness.  This should
-     * point to a statically-allocated structure, or NULL if unimplemented.
+     * must not depend on this functionality for correctness.
+     *
+     * Like the __GLXapiImports struct itself, this struct is allocated and
+     * zeroed by libGLX. The vendor library should assign function pointers
+     * to the members of this struct, but should not change the
+     * \c patchCallbacks pointer itself.
      */
-    const __GLdispatchPatchCallbacks *patchCallbacks;
+    __GLdispatchPatchCallbacks *patchCallbacks;
 
 } __GLXapiImports;
 
 /*****************************************************************************/
 
 #define __GLX_MAIN_PROTO_NAME "__glx_Main"
-#define __GLX_MAIN_PROTO(version, exports, vendorName)                \
-    const __GLXapiImports *__glx_Main(uint32_t version,               \
+#define __GLX_MAIN_PROTO(version, exports, vendor, imports)                \
+    __GLXapiImports *__glx_Main(uint32_t version,               \
                                       const __GLXapiExports *exports, \
-                                      __GLXvendorInfo *vendor)
+                                      __GLXvendorInfo *vendor, \
+                                      __GLXapiImports *imports)
 
-typedef const __GLXapiImports *(*__PFNGLXMAINPROC)
-    (uint32_t version, const __GLXapiExports *exports, __GLXvendorInfo *vendor);
+typedef Bool (*__PFNGLXMAINPROC)
+    (uint32_t version, const __GLXapiExports *exports, __GLXvendorInfo *vendor, __GLXapiImports *imports);
 
 /*!
  * Vendor libraries must export a function called __glx_Main() with the
@@ -328,23 +333,27 @@ typedef const __GLXapiImports *(*__PFNGLXMAINPROC)
  * Vendor libraries can optionally use the version number to support older
  * versions of the ABI.
  *
- * \param version The ABI version. The upper 16 bits contains the major version
+ * \param[in] version The ABI version. The upper 16 bits contains the major version
  * number, and the lower 16 bits contains the minor version number.
  *
- * \param exports The table of functions provided by libGLX. This pointer will
+ * \param[in] exports The table of functions provided by libGLX. This pointer will
  * remain valid for as long as the vendor is loaded.
  *
- * \param vendor The opaque pointer used to identify this vendor library. This
+ * \param[in] vendor The opaque pointer used to identify this vendor library. This
  * may be used in future versions to provide additional per-vendor information.
  *
- * \return A pointer to a __GLXapiImports struct, which must remain valid for
- * as long as the vendor is loaded. If the vendor library does not support the
- * requested ABI version, or if some other error occurrs, then it should return
- * \c NULL.
+ * \param[out] imports The function table that the vendor library should fill
+ * in. The vendor library must assign every non-optional function in the
+ * struct.
+ *
+ * \return True on success. If the vendor library does not support the
+ * requested ABI version or if some other error occurs, then it should return
+ * False.
  */
-const __GLXapiImports *__glx_Main(uint32_t version,
+Bool __glx_Main(uint32_t version,
                                   const __GLXapiExports *exports,
-                                  __GLXvendorInfo *vendor);
+                                  __GLXvendorInfo *vendor,
+                                  __GLXapiImports *imports);
 
 /*!
  * @}
diff --git a/src/GLX/libglxmapping.c b/src/GLX/libglxmapping.c
index a00e411..40f36bc 100644
--- a/src/GLX/libglxmapping.c
+++ b/src/GLX/libglxmapping.c
@@ -447,6 +447,37 @@ static GLboolean LookupVendorEntrypoints(__GLXvendorInfo *vendor)
     return GL_TRUE;
 }
 
+static Bool CheckVendorImportsTable(__GLXvendorNameHash *pEntry)
+{
+    // Make sure all the required functions are there.
+    if (pEntry->imports.isScreenSupported == NULL
+            || pEntry->imports.getProcAddress == NULL
+            || pEntry->imports.getDispatchAddress == NULL
+            || pEntry->imports.setDispatchIndex == NULL)
+    {
+        return False;
+    }
+
+    // Check to see whether this vendor library can support entrypoint
+    // patching.
+    if (pEntry->imports.patchCallbacks != NULL) {
+        if (pEntry->imports.patchCallbacks != &pEntry->patchCallbacks) {
+            // The vendor library shouldn't replace the patchCallbacks
+            // pointer. If it does, then just ignore it.
+            pEntry->imports.patchCallbacks = NULL;
+        } else {
+            if (pEntry->patchCallbacks.isPatchSupported == NULL
+                    || pEntry->patchCallbacks.initiatePatch == NULL) {
+                // If we don't have at least the isPatchSupported and
+                // initiatePatch callbacks, then patching isn't
+                // supported.
+                pEntry->imports.patchCallbacks = NULL;
+            }
+        }
+    }
+    return True;
+}
+
 static void *VendorGetProcAddressCallback(const char *procName, void *param)
 {
     __GLXvendorInfo *vendor = (__GLXvendorInfo *) param;
@@ -481,7 +512,7 @@ __GLXvendorInfo *__glXLookupVendorByName(const char *vendorName)
             __GLXvendorInfo *vendor;
             __PFNGLXMAINPROC glxMainProc;
             char *filename;
-            const __GLXapiImports *imports;
+            Bool success;
 
             // Previously unseen vendor. dlopen() the new vendor and add it to the
             // hash table.
@@ -522,30 +553,23 @@ __GLXvendorInfo *__glXLookupVendorByName(const char *vendorName)
 
             // Plug in the vendor imports table.
             pEntry->imports.patchCallbacks = &pEntry->patchCallbacks;
-            vendor->glxvc = &pEntry->imports;
 
             /* Initialize the dynamic dispatch table */
             LKDHASH_INIT(vendor->dynDispatchHash);
 
-            imports = (*glxMainProc)(GLX_VENDOR_ABI_VERSION,
+            success = (*glxMainProc)(GLX_VENDOR_ABI_VERSION,
                                       &glxExportsTable,
-                                      vendor);
-            if (!imports) {
+                                      vendor, &pEntry->imports);
+            if (!success) {
                 goto fail;
             }
 
-            // Copy the imports table from the vendor library.
-            memcpy(&pEntry->imports, imports, sizeof(__GLXapiImports));
-            if (imports->patchCallbacks != NULL) {
-                memcpy(&pEntry->patchCallbacks, imports->patchCallbacks, sizeof(__GLdispatchPatchCallbacks));
-            }
-            // Set the patchCallbacks table.
-            if (pEntry->patchCallbacks.isPatchSupported != NULL
-                    && pEntry->patchCallbacks.initiatePatch != NULL) {
-                pEntry->imports.patchCallbacks = &pEntry->patchCallbacks;
-            } else {
-                pEntry->imports.patchCallbacks = NULL;
+            // Check to see whether this vendor library can support entrypoint
+            // patching.
+            if (!CheckVendorImportsTable(pEntry)) {
+                goto fail;
             }
+            vendor->glxvc = &pEntry->imports;
 
             if (!LookupVendorEntrypoints(vendor)) {
                 goto fail;
diff --git a/src/GLdispatch/GLdispatch.c b/src/GLdispatch/GLdispatch.c
index f85b353..18fb07a 100644
--- a/src/GLdispatch/GLdispatch.c
+++ b/src/GLdispatch/GLdispatch.c
@@ -563,7 +563,9 @@ static int PatchEntrypoints(
     if (stubCurrentPatchCb) {
         // Notify the previous vendor that it no longer owns these
         // entrypoints.
-        stubCurrentPatchCb->releasePatch();
+        if (stubCurrentPatchCb->releasePatch != NULL) {
+            stubCurrentPatchCb->releasePatch();
+        }
     }
 
     if (patchCb) {
diff --git a/tests/GLX_dummy/GLX_dummy.c b/tests/GLX_dummy/GLX_dummy.c
index 16f4c0c..f8b6347 100644
--- a/tests/GLX_dummy/GLX_dummy.c
+++ b/tests/GLX_dummy/GLX_dummy.c
@@ -664,37 +664,27 @@ static GLboolean dummyInitiatePatch(int type,
     return GL_TRUE;
 }
 
-static void dummyReleasePatch(void)
-{
-}
-
-static const __GLdispatchPatchCallbacks dummyPatchCallbacks =
-{
-    .isPatchSupported = dummyCheckPatchSupported,
-    .initiatePatch = dummyInitiatePatch,
-    .releasePatch = dummyReleasePatch,
-};
 #endif // defined(PATCH_ENTRYPOINTS)
 
-static const __GLXapiImports dummyImports =
+PUBLIC Bool __glx_Main(uint32_t version,
+                                  const __GLXapiExports *exports,
+                                  __GLXvendorInfo *vendor,
+                                  __GLXapiImports *imports)
 {
-    .isScreenSupported = dummyCheckSupportsScreen,
-    .getProcAddress = dummyGetProcAddress,
-    .getDispatchAddress = dummyGetDispatchAddress,
-    .setDispatchIndex = dummySetDispatchIndex,
+    if (version <= GLX_VENDOR_ABI_VERSION) {
+        memcpy(&apiExports, exports, sizeof(*exports));
+
+        imports->isScreenSupported = dummyCheckSupportsScreen;
+        imports->getProcAddress = dummyGetProcAddress;
+        imports->getDispatchAddress = dummyGetDispatchAddress;
+        imports->setDispatchIndex = dummySetDispatchIndex;
 #if defined(PATCH_ENTRYPOINTS)
-    .patchCallbacks = &dummyPatchCallbacks,
-#else
-    .patchCallbacks = NULL,
+        imports->patchCallbacks->isPatchSupported = dummyCheckPatchSupported;
+        imports->patchCallbacks->initiatePatch = dummyInitiatePatch;
 #endif
-};
 
-PUBLIC __GLX_MAIN_PROTO(version, exports, vendor)
-{
-    if (version <= GLX_VENDOR_ABI_VERSION) {
-        memcpy(&apiExports, exports, sizeof(*exports));
-        return &dummyImports;
+        return True;
     } else {
-        return NULL;
+        return False;
     }
 }