loader: Remove a silly layer of reference counting

libdl will refcount objects for us just fine, thanks.

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Adam Jackson <ajax@redhat.com>

hw/xfree86/loader/loader.c

@@ -96,41 +96,6 @@
 
 extern void *xorg_symbols[];
 
-#define MAX_HANDLE 256
-static int refCount[MAX_HANDLE];
-
-/* Prototypes for static functions. */
-static loaderPtr listHead = NULL;
-
-static loaderPtr
-_LoaderListPush(void)
-{
-    loaderPtr item = calloc(1, sizeof(struct _loader));
-
-    item->next = listHead;
-    listHead = item;
-
-    return item;
-}
-
-static loaderPtr
-_LoaderListPop(int handle)
-{
-    loaderPtr item = listHead;
-    loaderPtr *bptr = &listHead;	/* pointer to previous node */
-
-    while (item) {
-	if (item->handle == handle) {
-	    *bptr = item->next;	/* remove this from the list */
-	    return item;
-	}
-	bptr = &(item->next);
-	item = item->next;
-    }
-
-    return 0;
-}
-
 void
 LoaderInit(void)
 {
@@ -171,130 +136,41 @@ LoaderInit(void)
 #endif
 }
 
-static void *
-do_dlopen(loaderPtr modrec, int flags)
-{
-    void *dlfile;
-    int dlopen_flags;
-
-    if (flags & LD_FLAG_GLOBAL)
-	dlopen_flags = DLOPEN_LAZY | DLOPEN_GLOBAL;
-    else
-	dlopen_flags = DLOPEN_LAZY;
-
-    dlfile = dlopen(modrec->name, dlopen_flags);
-
-    if (dlfile == NULL) {
-	ErrorF("dlopen: %s\n", dlerror());
-	return NULL;
-    }
-
-    return dlfile;
-}
-
 /* Public Interface to the loader. */
 
-int
+void *
-LoaderOpen(const char *module, int *errmaj, int *errmin, int *wasLoaded,
+LoaderOpen(const char *module, int *errmaj, int *errmin)
-           int flags)
 {
-    loaderPtr tmp;
+    void *ret;
-    int new_handle;
 
 #if defined(DEBUG)
     ErrorF("LoaderOpen(%s)\n", module);
 #endif
 
-    /* Is the module already loaded? */
-    tmp = listHead;
-    while (tmp) {
-#ifdef DEBUGLIST
-        ErrorF("strcmp(%x(%s),{%x} %x(%s))\n", module, module,
-               &(tmp->name), tmp->name, tmp->name);
-#endif
-        if (!strcmp(module, tmp->name)) {
-            refCount[tmp->handle]++;
-            if (wasLoaded)
-                *wasLoaded = 1;
-            xf86MsgVerb(X_INFO, 2, "Reloading %s\n", module);
-            return tmp->handle;
-        }
-        tmp = tmp->next;
-    }
-
-    /*
-     * OK, it's a new one. Add it.
-     */
     xf86Msg(X_INFO, "Loading %s\n", module);
-    if (wasLoaded)
-	*wasLoaded = 0;
 
-    /*
+    if (!(ret = dlopen(module, DLOPEN_LAZY | DLOPEN_GLOBAL))) {
-     * Find a free handle.
+	xf86Msg(X_ERROR, "Failed to load %s: %s\n", module, dlerror());
-     */
-    new_handle = 1;
-    while (new_handle < MAX_HANDLE && refCount[new_handle])
-	new_handle++;
-
-    if (new_handle == MAX_HANDLE) {
-	xf86Msg(X_ERROR, "Out of loader space\n");	/* XXX */
-	if (errmaj)
-	    *errmaj = LDR_NOSPACE;
-	if (errmin)
-	    *errmin = LDR_NOSPACE;
-	return -1;
-    }
-
-    refCount[new_handle] = 1;
-
-    tmp = _LoaderListPush();
-    tmp->name = strdup(module);
-    tmp->handle = new_handle;
-
-    if ((tmp->private = do_dlopen(tmp, flags)) == NULL) {
-	xf86Msg(X_ERROR, "Failed to load %s\n", module);
-	_LoaderListPop(new_handle);
-	refCount[new_handle] = 0;
 	if (errmaj)
 	    *errmaj = LDR_NOLOAD;
 	if (errmin)
 	    *errmin = LDR_NOLOAD;
-	return -1;
+	return NULL;
     }
 
-    return new_handle;
+    return ret;
-}
-
-int
-LoaderHandleOpen(int handle)
-{
-    if (handle < 0 || handle >= MAX_HANDLE)
-	return -1;
-
-    if (!refCount[handle])
-	return -1;
-
-    refCount[handle]++;
-    return handle;
 }
 
 void *
 LoaderSymbol(const char *name)
 {
     static void *global_scope = NULL;
-    loaderPtr l;
     void *p;
 
     p = dlsym(RTLD_DEFAULT, name);
     if (p != NULL)
 	return p;
 
-    for (l = listHead; l != NULL; l = l->next) {
-        p = dlsym(l->private, name);
-	if (p)
-	    return p;
-    }
-
     if (!global_scope)
 	global_scope = dlopen(NULL, DLOPEN_LAZY | DLOPEN_GLOBAL);
 
@@ -304,32 +180,11 @@ LoaderSymbol(const char *name)
     return NULL;
 }
 
-int
+void
-LoaderUnload(int handle)
+LoaderUnload(const char *name, void *handle)
 {
-    loaderRec fakeHead;
+    xf86Msg(X_INFO, "Unloading %s\n", name);
-    loaderPtr tmp = &fakeHead;
+    dlclose(handle);
-
-    if (handle < 0 || handle >= MAX_HANDLE)
-	return -1;
-
-    /*
-     * check the reference count, only free it if it goes to zero
-     */
-    if (--refCount[handle])
-	return 0;
-    /*
-     * find the loaderRecs associated with this handle.
-     */
-
-    while ((tmp = _LoaderListPop(handle)) != NULL) {
-	xf86Msg(X_INFO, "Unloading %s\n", tmp->name);
-	dlclose(tmp->private);
-	free(tmp->name);
-	free(tmp);
-    }
-
-    return 0;
 }
 
 unsigned long LoaderOptions = 0;

hw/xfree86/loader/loader.h

@@ -57,21 +57,6 @@
 #include <X11/Xfuncproto.h>
 #include <X11/Xmd.h>
 
-/* LoadModule proc flags; LD_FLAG_GLOBAL adds symbols to global
- * namespace, default is to keep symbols local to module. */
-#define LD_FLAG_GLOBAL 1
-
-typedef struct _loader *loaderPtr;
-
-/* Each module loaded has a loaderRec */
-typedef struct _loader {
-    int handle;			/* Unique id used to remove symbols from
-				 * this module when it is unloaded */
-    char *name;
-    void *private;		/* format specific data */
-    loaderPtr next;
-} loaderRec;
-
 /* Compiled-in version information */
 typedef struct {
     int xf86Version;
@@ -86,7 +71,6 @@ extern const ModuleVersions LoaderVersionInfo;
 extern unsigned long LoaderOptions;
 
 /* Internal Functions */
-int LoaderOpen(const char *, int *, int *, int *, int);
+void * LoaderOpen(const char *, int *, int *);
-int LoaderHandleOpen(int);
 
 #endif /* _LOADER_H */

hw/xfree86/loader/loaderProcs.h

@@ -60,7 +60,8 @@ typedef struct module_desc {
     struct module_desc *sib;
     struct module_desc *parent;
     char *name;
-    int handle;
+    char *path;
+    void *handle;
     ModuleSetupProc SetupProc;
     ModuleTearDownProc TearDownProc;
     void *TearDownData;		/* returned from SetupProc */
@@ -81,7 +82,7 @@ void UnloadDriver(ModuleDescPtr);
 void LoaderSetPath(const char *path);
 void LoaderSortExtensions(void);
 
-int LoaderUnload(int);
+void LoaderUnload(const char *, void *);
 unsigned long LoaderGetModuleVersion(ModuleDescPtr mod);
 
 void LoaderResetOptions(void);

hw/xfree86/loader/loadmod.c

@@ -83,8 +83,7 @@ static char *LoaderGetCanonicalName(const char *, PatternPtr);
 static void RemoveChild(ModuleDescPtr);
 static ModuleDescPtr doLoadModule(const char *, const char *, const char **,
 				  const char **, pointer,
-				  const XF86ModReqInfo *, int *, int *,
+				  const XF86ModReqInfo *, int *, int *);
-				  int flags);
 
 const ModuleVersions LoaderVersionInfo = {
     XORG_VERSION_CURRENT,
@@ -765,7 +764,7 @@ LoadSubModule(pointer _parent, const char *module,
     }
 
     submod = doLoadModule(module, NULL, subdirlist, patternlist, options,
-			  modreq, errmaj, errmin, LD_FLAG_GLOBAL);
+			  modreq, errmaj, errmin);
     if (submod && submod != (ModuleDescPtr) 1) {
 	parent->child = AddSibling(parent->child, submod);
 	submod->parent = parent;
@@ -776,18 +775,10 @@ LoadSubModule(pointer _parent, const char *module,
 static ModuleDescPtr
 NewModuleDesc(const char *name)
 {
-    ModuleDescPtr mdp = malloc(sizeof(ModuleDesc));
+    ModuleDescPtr mdp = calloc(1, sizeof(ModuleDesc));
 
-    if (mdp) {
+    if (mdp)
-	mdp->child = NULL;
-	mdp->sib = NULL;
-	mdp->parent = NULL;
 	mdp->name = xstrdup(name);
-	mdp->handle = -1;
-	mdp->SetupProc = NULL;
-	mdp->TearDownProc = NULL;
-	mdp->TearDownData = NULL;
-    }
 
     return mdp;
 }
@@ -796,6 +787,7 @@ ModuleDescPtr
 DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent)
 {
     ModuleDescPtr ret;
+    int errmaj, errmin;
 
     if (!mod)
 	return NULL;
@@ -804,10 +796,11 @@ DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent)
     if (ret == NULL)
 	return NULL;
 
-    if (LoaderHandleOpen(mod->handle) == -1)
+    if (!(ret->handle = LoaderOpen(mod->path, &errmaj, &errmin))) {
+        free(ret);
         return NULL;
+    }
 
-    ret->handle = mod->handle;
     ret->SetupProc = mod->SetupProc;
     ret->TearDownProc = mod->TearDownProc;
     ret->TearDownData = NULL;
@@ -815,6 +808,7 @@ DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent)
     ret->sib = DuplicateModule(mod->sib, parent);
     ret->parent = parent;
     ret->VersionInfo = mod->VersionInfo;
+    ret->path = strdup(mod->path);
 
     return ret;
 }
@@ -830,7 +824,7 @@ static ModuleDescPtr
 doLoadModule(const char *module, const char *path, const char **subdirlist,
 	     const char **patternlist, pointer options,
 	     const XF86ModReqInfo * modreq,
-	     int *errmaj, int *errmin, int flags)
+	     int *errmaj, int *errmin)
 {
     XF86ModuleData *initdata = NULL;
     char **pathlist = NULL;
@@ -839,7 +833,6 @@ doLoadModule(const char *module, const char *path, const char **subdirlist,
     char **path_elem = NULL;
     char *p = NULL;
     ModuleDescPtr ret = NULL;
-    int wasLoaded = 0;
     PatternPtr patterns = NULL;
     int noncanonical = 0;
     char *m = NULL;
@@ -926,9 +919,10 @@ doLoadModule(const char *module, const char *path, const char **subdirlist,
 	    *errmin = 0;
 	goto LoadModule_fail;
     }
-    ret->handle = LoaderOpen(found, errmaj, errmin, &wasLoaded, flags);
+    ret->handle = LoaderOpen(found, errmaj, errmin);
     if (ret->handle < 0)
 	goto LoadModule_fail;
+    ret->path = strdup(found);
 
     /* drop any explicit suffix from the module name */
     p = strchr(name, '.');
@@ -959,7 +953,6 @@ doLoadModule(const char *module, const char *path, const char **subdirlist,
 	setup = initdata->setup;
 	teardown = initdata->teardown;
 
-	if (!wasLoaded) {
         if (vers) {
             if (!CheckVersion(module, vers, modreq)) {
                 if (errmaj)
@@ -978,7 +971,6 @@ doLoadModule(const char *module, const char *path, const char **subdirlist,
                 *errmin = 0;
             goto LoadModule_fail;
         }
-	}
 	if (setup)
 	    ret->SetupProc = setup;
 	if (teardown)
@@ -1066,7 +1058,7 @@ LoadModule(const char *module, const char *path, const char **subdirlist,
 	   const XF86ModReqInfo * modreq, int *errmaj, int *errmin)
 {
   return doLoadModule(module, path, subdirlist, patternlist, options,
-		      modreq, errmaj, errmin, LD_FLAG_GLOBAL);
+		      modreq, errmaj, errmin);
 }
 
 void
@@ -1088,12 +1080,13 @@ UnloadModuleOrDriver(ModuleDescPtr mod)
 
     if ((mod->TearDownProc) && (mod->TearDownData))
 	mod->TearDownProc(mod->TearDownData);
-    LoaderUnload(mod->handle);
+    LoaderUnload(mod->name, mod->handle);
 
     if (mod->child)
 	UnloadModuleOrDriver(mod->child);
     if (mod->sib)
 	UnloadModuleOrDriver(mod->sib);
+    free(mod->path);
     free(mod->name);
     free(mod);
 }
@@ -1110,13 +1103,14 @@ UnloadSubModule(pointer _mod)
 
     if ((mod->TearDownProc) && (mod->TearDownData))
 	mod->TearDownProc(mod->TearDownData);
-    LoaderUnload(mod->handle);
+    LoaderUnload(mod->name, mod->handle);
 
     RemoveChild(mod);
 
     if (mod->child)
 	UnloadModuleOrDriver(mod->child);
 
+    free(mod->path);
     free(mod->name);
     free(mod);
 }