Re: 277-gary-rename-remaining-troublesome-ltdl-apis.diff

[Gary V. Vaughan]

Ralf Wildenhues wrote:
> Hi Gary,

Hallo Ralf,

> * Gary V. Vaughan wrote on Sun, Sep 18, 2005 at 02:30:03AM CEST:
> > This fixes the remaining holes that allowed libltdl clients to stumble
> > across each others' loaded modules.  Of course, there is nothing to
> > prevent clients from registering a dumb interface validator that returns
> > other clients' modules... hopefully the extra stuff I've added to the
> > manual makes it clear how to do it properly though. 
>
> Huh?  Once again I fail to grasp why you want to remove functionality.
> AFAICS lt_dlforeach, lt_dlhandle_next are the ones present in 1.5.x, the
> other to-be-killed ones were added later.  Is this correct?

Yes.

Code written with lt_dlforeach and lt_dlhandle_next semantics (presumably
doing something to each loaded module, which with 1.5x were all property
of the callers of these APIs -- because all modules had to be loaded
from the main application), will end up iterating through all modules
loaded by libltdl, including the module loader modules from libltdl, and
any modules loaded (or preloaded) by third party libraries.  This is
bad, because libltdl base module loaders can now start unloading or
otherwise breaking each others' modules.

The only way to help an ltdl client to iterate over its own modules only
is to require an lt_dlinterface_id argument, but as we discussed earlier
simply adding an argument to an existing entry point is less likely to
alert the user to the fact their code is about to do something horribly
wrong.  I've added the new argument and renamed the old functions to
force a hard error to save users from this problem, and then removed the
kludgy bits that were added in the trunk to try and keep source
compatibility (which is the wrong thing to do here anyway).

> The new stuff looks, umm, well, new to me.  As in: would need a bit of
> real-world experience to prove that "this new interface can finally live
> up to user's needs", which apparently all the previous versions could
> not.  So, no, I'm not convinced by either removing the old interfaces
> nor adding the new ones, although the new ones do not look suspicious
> per se.  Also, we have no code that tests the new interfaces at all.
> Not even CVS m4.

I don't want to commit changes to CVS m4 for patches that aren't yet
accepted into CVS libtool.  I've attached the diff of what needs to be
done to bring m4 in line though. (I fixed a handful of latent bugs by
manually updating like this btw, and I expect our users will have the
same experience).

> lt_dlhandle_next already has the machinery to do the right thing.  Right?

Wrong.  It can't be stopped from walking across everyone elses modules,
and handing out your modules to any other library that uses
lt_dlhandle_next.

> I should reiterate Bob's statement: only _minimal_ changes should be
> necessary to stabilize.  Sarcastically, I should add: For every
> interface that is removed, both the person originally creating the
> interface, and, to lesser extent, the one removing it should be
> punished.  :->

Better that than punishing all libltdl users! :-p

> > Once this is committed, we can remove another of the remaining release
> > blockers... :-D
>
> AFAICS that was already done with 275 and 276.  No need for this one.
> Am I missing something again?

Not quite... Hope this post clarifies!

Cheers,
        Gary.

> > +2005-09-18  Gary V. Vaughan  <address@hidden>
> > +
> > +       * libltdl/ltdl.h, libltdl/ltdl.c (lt_dlhandle_first): Removed.
> > +       * libltdl/ltdl.h, libltdl/ltdl.c (lt_dlhandle_next)
> > +       (lt_dlhandle_find, lt_dlforeach): Removed...
> > +       (lt_dlhandle_iterate, lt_dlhandle_fetch, lt_dlhandle_map): Similar
> > +       functions that are multi-loader safe, and require a registered
> > +       interface validator argument.
> > +       * doc/libtool.texi: Updated.
> > +       * NEWS: Updated.
--
Gary V. Vaughan      ())_.  address@hidden,gnu.org}
Research Scientist   ( '/   http://tkd.kicks-ass.net
GNU Hacker           / )=   http://www.gnu.org/software/libtool
Technical Author   `(_~)_   http://sources.redhat.com/autobook

 m4/builtin.c   |   10 +++++-----
 m4/m4private.h |    5 +++--
 m4/module.c    |   41 +++++++++++++++++++++++++++--------------
 modules/load.c |    6 +++---
 src/freeze.c   |    7 ++++---
 5 files changed, 42 insertions(+), 27 deletions(-)

Index: m4--devo--1.0/ChangeLog
from  Gary V. Vaughan  <address@hidden>
        * m4/module.c (caller_id): To match libtool-2.0 interface, changed
        to ...
        (iface_id): ...an lt_dlinterface_id type.
        (m4__module_find): New abstraction for lt_dlhandle_fetch.  Use
        throughout, instead of calling obsolete lt_dlhandle_find directly.
        (m4__module_next): Use multiloader-safe lt_dlhandle_iterate.  Use
        throughout, instead of calling obsolete lt_dlhandle_next.
        * m4/m4private.h (m4__module_find): Declare it.
        * m4/builtin.c (m4_builtin_find_by_name, m4_builtin_find_by_func):
        Use m4__module_next instead of obsolete lt_dlhandle_next.
Index: m4--devo--1.0/m4/builtin.c
===================================================================
--- m4--devo--1.0.orig/m4/builtin.c
+++ m4--devo--1.0/m4/builtin.c
@@ -1,5 +1,5 @@
 /* GNU m4 -- A simple macro processor
-   Copyright (C) 1989-1994, 1999, 2000 Free Software Foundation, Inc.
+   Copyright (C) 1989-1994, 1999, 2000, 2005 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -27,7 +27,7 @@
 const m4_builtin *
 m4_builtin_find_by_name (lt_dlhandle handle, const char *name)
 {
-  lt_dlhandle cur = handle ? handle : lt_dlhandle_next (0);
+  lt_dlhandle cur = handle ? handle : m4__module_next (0);
 
   do
     {
@@ -41,7 +41,7 @@ m4_builtin_find_by_name (lt_dlhandle han
              return builtin;
        }
     }
-  while (!handle && (cur = lt_dlhandle_next (cur)));
+  while (!handle && (cur = m4__module_next (cur)));
 
   return 0;
 }
@@ -51,7 +51,7 @@ m4_builtin_find_by_name (lt_dlhandle han
 const m4_builtin *
 m4_builtin_find_by_func (lt_dlhandle handle, m4_builtin_func *func)
 {
-  lt_dlhandle cur = handle ? handle : lt_dlhandle_next (0);
+  lt_dlhandle cur = handle ? handle : m4__module_next (0);
 
   do
     {
@@ -65,7 +65,7 @@ m4_builtin_find_by_func (lt_dlhandle han
              return builtin;
        }
     }
-  while (!handle && (cur = lt_dlhandle_next (cur)));
+  while (!handle && (cur = m4__module_next (cur)));
 
   return 0;
 }
Index: m4--devo--1.0/m4/m4private.h
===================================================================
--- m4--devo--1.0.orig/m4/m4private.h
+++ m4--devo--1.0/m4/m4private.h
@@ -1,7 +1,7 @@
 /* GNU m4 -- A simple macro processor
 
-   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 1998, 1999, 2004,
-   2005 Free Software Foundation, Inc.
+   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 1998, 1999, 2004, 2005
+                 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -137,6 +137,7 @@ extern lt_dlhandle  m4__module_open (m4 
                                     m4_obstack *obs);
 extern void        m4__module_exit (m4 *context);
 extern lt_dlhandle  m4__module_next (lt_dlhandle);
+extern lt_dlhandle  m4__module_find (const char *name);
 
 
 
Index: m4--devo--1.0/m4/module.c
===================================================================
--- m4--devo--1.0.orig/m4/module.c
+++ m4--devo--1.0/m4/module.c
@@ -1,5 +1,6 @@
 /* GNU m4 -- A simple macro processor
-   Copyright (C) 1989-1994, 1998, 1999, 2002, 2003, 2004 Free Software 
Foundation, Inc.
+   Copyright (C) 1989-1994, 1998, 1999, 2002, 2003, 2004, 2005
+                 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -90,7 +91,7 @@ static const m4_macro *   install_macro_
 static int         m4__module_interface        (lt_dlhandle handle,
                                                 const char *id_string);
 
-static lt_dlcaller_id caller_id = 0;
+static lt_dlinterface_id iface_id = 0;
 
 const char *
 m4_get_module_name (lt_dlhandle handle)
@@ -108,7 +109,7 @@ void *
 m4_module_import (m4 *context, const char *module_name,
                  const char *symbol_name, m4_obstack *obs)
 {
-  lt_dlhandle  handle          = lt_dlhandle_find (module_name);
+  lt_dlhandle  handle          = m4__module_find (module_name);
   lt_ptr       symbol_address  = 0;
 
   /* Try to load the module if it is not yet available (errors are
@@ -253,7 +254,7 @@ m4_module_unload (m4 *context, const cha
   assert (context);
 
   if (name)
-    handle = lt_dlhandle_find (name);
+    handle = m4__module_find (name);
 
   if (!handle)
     {
@@ -287,11 +288,23 @@ m4__module_interface (lt_dlhandle handle
 
 
 /* Return successive loaded modules that pass the interface test registered
-   with the caller id.  */
+   with the interface id.  */
 lt_dlhandle
 m4__module_next (lt_dlhandle handle)
 {
-  return handle ? lt_dlhandle_next (handle) : lt_dlhandle_first (caller_id);
+  assert (iface_id);
+
+  return lt_dlhandle_iterate (iface_id, handle);
+}
+
+/* Return the first loaded module that passes the registered interface test
+   and is called NAME.  */
+lt_dlhandle
+m4__module_find (const char *name)
+{
+  assert (iface_id);
+
+  return lt_dlhandle_fetch (iface_id, name);
 }
 
 
@@ -304,9 +317,9 @@ m4__module_init (m4 *context)
 {
   int errors = 0;
 
-  /* Do this only once!  If we already have a caller_id, then the
+  /* Do this only once!  If we already have an iface_id, then the
      module system has already been initialised.  */
-  if (caller_id)
+  if (iface_id)
     {
       M4ERROR ((m4_get_warning_status_opt (context), 0,
                _("Warning: multiple module loader initialisations")));
@@ -319,9 +332,9 @@ m4__module_init (m4 *context)
      ltdl module handles.  */
   if (!errors)
     {
-      caller_id = lt_dlcaller_register ("m4 libm4", m4__module_interface);
+      iface_id = lt_dlinterface_register ("m4 libm4", m4__module_interface);
 
-      if (!caller_id)
+      if (!iface_id)
        {
          const char *error_msg = _("libltdl client registration failed");
 
@@ -369,7 +382,7 @@ m4__module_open (m4 *context, const char
   m4_module_init_func *        init_func       = 0;
 
   assert (context);
-  assert (caller_id);
+  assert (iface_id);           /* need to have called m4__module_init */
 
   if (handle)
     {
@@ -423,18 +436,18 @@ m4__module_open (m4 *context, const char
 void
 m4__module_exit (m4 *context)
 {
-  lt_dlhandle  handle  = lt_dlhandle_first (caller_id);
+  lt_dlhandle  handle  = m4__module_next (0);
   int          errors  = 0;
 
   while (handle && !errors)
     {
+      const lt_dlinfo *info    = lt_dlgetinfo (handle);
       lt_dlhandle      pending = handle;
-      const lt_dlinfo *info    = lt_dlgetinfo (pending);
 
       /* If we are about to unload the final reference, move on to the
         next handle before we unload the current one.  */
       if (info->ref_count <= 1)
-       handle = lt_dlhandle_next (pending);
+       handle = m4__module_next (handle);
 
       errors = module_remove (context, pending, 0);
     }
Index: m4--devo--1.0/modules/load.c
===================================================================
--- m4--devo--1.0.orig/modules/load.c
+++ m4--devo--1.0/modules/load.c
@@ -1,5 +1,5 @@
 /* GNU m4 -- A simple macro processor
-   Copyright (C) 2000 Free Software Foundation, Inc.
+   Copyright (C) 2000, 2005 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -93,14 +93,14 @@ M4BUILTIN_HANDLER (modules)
 {
   /* The expansion of this builtin is a comma separated list of
      loaded modules.  */
-  lt_dlhandle handle = lt_dlhandle_next (NULL);
+  lt_dlhandle handle = m4__module_next (NULL);
 
   if (handle)
     do
       {
        m4_shipout_string (context, obs, m4_get_module_name (handle), 0, true);
 
-       if ((handle = lt_dlhandle_next (handle)))
+       if ((handle = m4__module_next (handle)))
          obstack_1grow (obs, ',');
       }
     while (handle);
Index: m4--devo--1.0/src/freeze.c
===================================================================
--- m4--devo--1.0.orig/src/freeze.c
+++ m4--devo--1.0/src/freeze.c
@@ -1,5 +1,6 @@
 /* GNU m4 -- A simple macro processor
-   Copyright (C) 1989, 90, 91, 92, 93, 94, 04 Free Software Foundation, Inc.
+   Copyright (C) 1989, 90, 91, 92, 93, 94, 2004, 2005
+                 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -463,7 +464,7 @@ reload_frozen_state (m4 *context, const 
          lt_dlhandle handle   = 0;
 
          if (number[2] > 0)
-           handle = lt_dlhandle_find (string[2]);
+           handle = m4__module_find (string[2]);
 
          if (handle)
            bp = m4_builtin_find_by_name (handle, string[1]);
@@ -660,7 +661,7 @@ reload_frozen_state (m4 *context, const 
          lt_dlhandle handle = 0;
 
          if (number[2] > 0)
-           handle = lt_dlhandle_find (string[2]);
+           handle = m4__module_find (string[2]);
 
          m4_set_symbol_value_text (token, xstrdup (string[1]));
          VALUE_HANDLE (token)          = handle;

signature.asc
Description: OpenPGP digital signature