HEAD: fix lt_dlloader_remove memleak in libltdl

[Ralf Wildenhues]

Problem 1)
lt_dlloader_remove currently always iterates over the empty set.  This
was communicated to me by Jeff, who found out by means of Coverity.
It leads to a small amount memory leaks, exposed in the mdemo test when
running valgrind.

Problem 2)
lt_dlloader_remove has encountered a signature change from branch-1-5
to HEAD: it returns a `lt_dlvtable *' now, rather than an int.
This change is not reflected in the manual nor in NEWS, also it is not
so clear how error semantics are (see below).  Similar problems are all
over the map in the loader @node of the manual, with several functions.
:-(

Problem 3)
Fixing (1) lets the lt_dladvise test exposed the weakness in the error
semantics of lt_dlloader_remove: if you have loaded any resident
modules, then lt_dlexit will now return an error (propagated from
lt_dlloader_remove).  This seems rather undesirable, because the user
cannot really do anything against it.


The patch below fixes (1) and (3), the latter by not trying to remove
loaders that are used for resident modules, and not setting an error in
this case.

The whole boogaboo I put in lt_dlloader.c with using the iterate API and
so on is to avoid exposing `loaders' to the world by making it
non-static, in which case the dlloaders.c code could use it directly.

OK to apply?

Cheers,
Ralf

2007-09-01  Ralf Wildenhues  <address@hidden>

        * libltdl/lt_dlloader.c (lt_dlloader_remove): Actually iterate
        over all open modules when looking for modules that use it.
        If a resident module is found, return but do not set the error
        string.  For iteration, use...
        (find_all_handles): ...this new trivial callback function.
        * libltdl/ltdl.c (lt_dlexit): When removing dlloaders, ignore
        errors that stem from earlier failed commands.  Exposed by
        lt_dladvise test.
        Fixes regression over branch-1-5.
        Memleak report as Coverity CID 19 via Jeff Squyres.

Index: libltdl/lt_dlloader.c
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/lt_dlloader.c,v
retrieving revision 1.11
diff -u -r1.11 lt_dlloader.c
--- libltdl/lt_dlloader.c       4 Jul 2007 23:05:04 -0000       1.11
+++ libltdl/lt_dlloader.c       1 Sep 2007 10:02:00 -0000
@@ -142,17 +142,28 @@
   return (const lt_dlvtable *) (loader ? ((SList *) loader)->userdata : 0);
 }
 
+/* Callback function to iterate over all handles */
+static int
+find_all_handles (lt_dlhandle LT__UNUSED handle, const char * LT__UNUSED id)
+{
+  return 0;
+}
 
 /* Return the contents of the first item in the global loader list
    with a matching NAME after removing it from that list.  If there
    was no match, return NULL; if there is an error, return NULL and
-   set an error for lt_dlerror; in either case, the loader list is
-   not changed if NULL is returned.  */
+   set an error for lt_dlerror; do not set an error if only resident
+   modules need this loader; in either case, the loader list is not
+   changed if NULL is returned.  */
 lt_dlvtable *
 lt_dlloader_remove (char *name)
 {
   const lt_dlvtable *  vtable  = lt_dlloader_find (name);
-  lt__handle *         handle  = 0;
+  static const char    id_string[] = "lt_dlloader_remove";
+  lt_dlinterface_id    iface;
+  lt_dlhandle          handle = 0;
+  int                  in_use = 0;
+  int                  in_use_by_resident = 0;
 
   if (!vtable)
     {
@@ -161,14 +172,24 @@
     }
 
   /* Fail if there are any open modules which use this loader.  */
-  for  (handle = 0; handle; handle = handle->next)
+  iface = lt_dlinterface_register (id_string, find_all_handles);
+  while ((handle = lt_dlhandle_iterate (iface, handle)))
     {
-      if (handle->vtable == vtable)
+      lt__handle *cur = (lt__handle *) handle;
+      if (cur->vtable == vtable)
        {
-         LT__SETERROR (REMOVE_LOADER);
-         return 0;
+         in_use = 1;
+         if (lt_dlisresident (handle))
+           in_use_by_resident = 1;
        }
     }
+  lt_dlinterface_free (iface);
+  if (in_use)
+    {
+      if (!in_use_by_resident)
+       LT__SETERROR (REMOVE_LOADER);
+      return 0;
+    }
 
   /* Call the loader finalisation function.  */
   if (vtable && vtable->dlloader_exit)
Index: libltdl/ltdl.c
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.260
diff -u -r1.260 ltdl.c
--- libltdl/ltdl.c      1 Sep 2007 08:13:20 -0000       1.260
+++ libltdl/ltdl.c      1 Sep 2007 10:02:01 -0000
@@ -310,6 +310,12 @@
            break;
        }
 
+      /* When removing loaders, we can only find out failure by testing
+        the error string, so avoid a spurious one from an earlier
+        failed command. */
+      if (!errors)
+       LT__SETERRORSTR (0);
+
       /* close all loaders */
       for (loader = (lt_dlloader *) lt_dlloader_next (NULL); loader;)
        {
@@ -322,7 +328,11 @@
            }
          else
            {
-             ++errors;
+             /* ignore errors due to resident modules */
+             const char *err;
+             LT__GETERROR (err);
+             if (err)
+               ++errors;
            }
 
          loader = next;