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 themanual 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... :-DAFAICS 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 <[EMAIL PROTECTED]> + + * 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 ())_. [EMAIL PROTECTED],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 <[EMAIL PROTECTED]>
* 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
