Hi Gary,

* Gary V. Vaughan wrote on Mon, Oct 10, 2005 at 12:26:25PM CEST:
>  ChangeLog        |    0 
>  NEWS             |    9 +-
>  doc/libtool.texi |  176 
> +++++++++++++++++++++----------------------------------
>  libltdl/ltdl.c   |   88 ++++++++++-----------------
>  libltdl/ltdl.h   |   10 +--
>  4 files changed, 114 insertions(+), 169 deletions(-)
> 
> Index: libtool--devo--1.0/ChangeLog
> from  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.

OK to apply, with the nits below addressed.
Please also apply the then-freed part of your patch queue,
that should 284, 285, 286 I believe; with 286, please just
take one look at the additional tiny nit I found, see other mail.

Thank you!

Cheers,
Ralf

> Index: libtool--devo--1.0/NEWS
> ===================================================================
> --- libtool--devo--1.0.orig/NEWS
> +++ libtool--devo--1.0/NEWS
> @@ -4,11 +4,12 @@ New in 1.9h: 2005-??-??; CVS version 2.1
>  * New tests for support of Automake subdir-objects.
>  * Fix libltdl on static platforms.
>  * New LT_CONFIG_LTDL_DIR macro.
> -* New lt_dlinterface_register, lt_dlinterface_set_data and
> -  lt_dlinterface_get_data libltdl API calls to maintain separation of
> -  concerns between modules loaded by different libraries.
> +* New multi-module-loader safe libltdl handle iteration APIs:
> +  lt_dlhandle_iterate, lt_dlhandle_fetch, lt_dlhandle_map.
> +* New lt_dlinterface_register to maintain separation of concerns between
> +  modules loaded by different libraries.
>  * Removed deprecated APIs from libltdl: lt_dlcaller_register,
> -  lt_dlcaller_get_data, lt_dlcaller_set_data, lt_dlmutex_register,
> +  lt_dlhandle_next, lt_dlhandle_find, lt_dlforeach, lt_dlmutex_register,
>    lt_dlmutex_lock, lt_dlmutex_unlock, lt_dlmutex_seterror,
>    lt_dlmutex_geterror, lt_dlmalloc, lt_dlrealloc, lt_dlfree.
>  * Support for Portland Group compiler on GNU/Linux.
> Index: libtool--devo--1.0/doc/libtool.texi
> ===================================================================
> --- libtool--devo--1.0.orig/doc/libtool.texi
> +++ libtool--devo--1.0/doc/libtool.texi
> @@ -3734,35 +3734,85 @@ Furthermore, in order to save you from h
>  handles of all the modules you have loaded, these functions allow you to
>  iterate over libltdl's list of loaded modules:
>  
> [EMAIL PROTECTED] int lt_dlforeach (@w{int ([EMAIL PROTECTED]) (lt_dlhandle 
> @var{handle}, void * @var{data})}, @w{void * @var{data}})
> -For each loaded module call the function @var{func}.  The argument
> [EMAIL PROTECTED] is the handle of one of the loaded modules, @var{data} is
> -the @var{data} argument passed to @code{lt_dlforeach}.
> -As soon as @var{func} returns a non-zero value for one of the handles,
> [EMAIL PROTECTED] will stop calling @var{func} and immediately return 1.
> -Otherwise 0 is returned.
> [EMAIL PROTECTED] {Type} lt_dlinterface_id
> +The opaque type used to hold the module interface details for each
> +registered libltdl client.
> [EMAIL PROTECTED] deftp
> +
> [EMAIL PROTECTED] {Type} int lt_dlhandle_interface (@w{lt_dlhandle 
> @var{handle},} @w{const char [EMAIL PROTECTED])

Hmm, this @deftp definition is rendered a bit weird in the DVI, but I
suppose that is either deliberate (by texinfo) or a limitation.  Dunno
what would be better here.

> +Functions of this type are called to check that a handle conforms to a
> +library's expected module interface when iterating over the global
> +handle list.  You should be careful to write a callback function of
> +this type that can correctly identify modules that belong to this
> +client, both to prevent other clients from accidentally finding your
> +loaded modules with the iterator functions below, and vice versa.  The
> +best way of doing this is to check that module @var{handle} conforms

better: `to do this'?  (just a thought)

> +to the interface specification of your loader using @code{lt_dlsym}.

This is pretty inefficient: the user will actually have to store a list
of loaded modules itself.  Hrmpf.

Ah, no, never mind.  Your example below clarifies it.
(Maybe above is a bit difficult to understand?)

> +
> +The callback will be given @strong{every} module loaded by all the
> +libltdl module clients in the current address space, including any

Can we weaken this to: `The callback may be given every module..'?
Rationale: we might change to use a more efficient implementation later
(or have to use it for unrelated reasons), which will allow ltdl itself
to weed out the other modules at less cost; no need to restrict
ourselves not to be able to pass on that improvement.

Just an idea; decide for yourself.

> +modules loaded by other libraries such as libltdl itself, and should
> +return non-zero if that module does not fulfill the interface
> +requirements of your loader.
> +
> [EMAIL PROTECTED]
> +int
> +my_interface_cb (lt_dlhandle handle, const char *id_string)
> [EMAIL PROTECTED]
> +  char *(*module_id) (void) = NULL;
> +
> +  /* @r{A valid my_module must provide all of these symbols.}  */
> +  if (!((module_id = (char*(*)(void)) lt_dlsym ("module_version"))
> +        && lt_dlsym ("my_module_entrypoint")))
> +      return 1;
> +
> +  if (strcmp (id_string, module_id()) != 0)
> +      return 1;
> +
> +  return 0;
> [EMAIL PROTECTED]
> [EMAIL PROTECTED] example
> [EMAIL PROTECTED] deftp
> +
> [EMAIL PROTECTED] lt_dlinterface_id lt_dlinterface_register (@w{const char 
> [EMAIL PROTECTED],} @w{lt_dlhandle_interface [EMAIL PROTECTED])

  s/},}/}},/

> +Use this function to register your interface validator with libltdl,
> +and in return obtain a unique key to store and retrieve per module data.

Maybe s/per module/per-module/ for easier understanding?

> +You supply an @var{id_string} and @var{iface} so that the resulting
> [EMAIL PROTECTED] can be used to filter the module handles
> +returned by the iteration functions below.
> [EMAIL PROTECTED] deftypefun
> +
> [EMAIL PROTECTED] int lt_dlhandle_map (@w{lt_dlinterface_id @var{iface}}, 
> @w{int ([EMAIL PROTECTED]) (lt_dlhandle @var{handle}, void * @var{data})}, 
> @w{void * @var{data}})
> +For each module that matches @var{iface}, call the function
> [EMAIL PROTECTED]  When writing the @var{func} callback function, the
> +argument @var{handle} is the handle of a loaded modules, and
> [EMAIL PROTECTED] is the @var{data} argument passed to

Easier to understand if the last [EMAIL PROTECTED]' is replaced by `last',
IMVHO.

> [EMAIL PROTECTED] As soon as @var{func} returns a non-zero value
> +for one of the handles, @code{lt_dlhandles_map} will stop calling

  s/lt_dlhandles_map/lt_dlhandle_map/

> [EMAIL PROTECTED] and immediately return 1.  Otherwise 0 is returned.
>  @end deftypefun

Wouldn't it be even more useful if it just returned the error code in
that case?  The user could carry some information that way, without the
hassle of protecting data in a concurrent execution setup, for example.

Incidentally, that would also make the implementation of lt_dlhandle_map
even simpler.

> [EMAIL PROTECTED] lt_dlhandle lt_dlhandle_next (@w{lt_dlhandle place})
> -Iterate over the loaded module handles, returning the first handle in the
> -list if @var{place} is @code{NULL}, and the next one on subsequent calls.
> -If @var{place} is the last element in the list of loaded modules, this
> -function returns @code{NULL}.
> [EMAIL PROTECTED] lt_dlhandle lt_dlhandle_iterate (@w{lt_dlinterface_id 
> @var{iface}}, @w{lt_dlhandle place})

  s/place/@var{&}/

> +Iterate over the module handles loaded by @var{iface}, returning the
> +first matching handle in the list if @var{place} is @code{NULL}, and
> +the next one on subsequent calls.  If @var{place} is the last element
> +in the list of eligible modules, this function returns @code{NULL}.
>  
>  @example
>  lt_dlhandle handle = 0;
> +lt_dlinterface_id iface = my_interface_id;
>  
> -while ((handle = lt_dlhandle_next (handle)))
> +while ((handle = lt_dlhandle_iterate (iface, handle)))
>    @{
>      @dots{}
>    @}
>  @end example
>  @end deftypefun
>  
> [EMAIL PROTECTED] lt_dlhandle lt_dlhandle_find (@w{const char [EMAIL 
> PROTECTED])
> -Search through the loaded module handles for a module named
> [EMAIL PROTECTED] lt_dlhandle lt_dlhandle_fetch (@w{lt_dlinterface_id 
> @var{iface}}, @w{const char [EMAIL PROTECTED])
> +Search through the module handles loaded by @var{iface} for a module named
>  @var{module_name}, returning its handle if found or else @code{NULL}
> -if no such named module has been loaded.
> +if no such named module has been loaded by @var{iface}.
>  @end deftypefun
>  
>  However, you might still need to maintain your own list of loaded
> @@ -3770,26 +3820,10 @@ module handles (in parallel with the lis
>  if there were any other data that your application wanted to associate
>  with each open module.  Instead, you can use the following @sc{api}
>  calls to do that for you.  You must first obtain a unique interface id
> -from libltdl, and subsequently always use it to retrieve the data you
> -stored earlier.  This allows different libraries to each store their
> -own data against loaded modules, without interfering with one another.
> -
> [EMAIL PROTECTED] {Type} lt_dlinterface_id
> -The opaque type used to hold individual data set keys.
> [EMAIL PROTECTED] deftp
> -
> [EMAIL PROTECTED] {Type} int lt_dlhandle_interface (@w{lt_dlhandle 
> @var{handle},} @w{const char [EMAIL PROTECTED])
> -Functions of this type are called to check that a handle conforms to a
> -library's expected module interface when iterating over the global
> -handle list.
> [EMAIL PROTECTED] deftp
> -
> [EMAIL PROTECTED] lt_dlinterface_id lt_dlinterface_register (@w{const char 
> [EMAIL PROTECTED],} @w{lt_dlhandle_interface [EMAIL PROTECTED])
> -Use this to obtain a unique key to store and retrieve per module data,
> -if you supply an @var{id_string} and @var{iface}, then the resulting
> [EMAIL PROTECTED] can be used to filter the module handles
> -returned by @samp{lt_dlhandle_next}.
> [EMAIL PROTECTED] deftypefun
> +from libltdl as described above, and subsequently always use it to
> +retrieve the data you stored earlier.  This allows different libraries
> +to each store their own data against loaded modules, without
> +interfering with one another.
>  
>  @deftypefun {void *} lt_dlcaller_set_data (@w{lt_dlinterface_id @var{key}}, 
> @w{lt_dlhandle @var{handle}}, @w{void * @var{data}})
>  Set @var{data} as the set of data uniquely associated with @var{key} and
> @@ -3824,76 +3858,6 @@ Return the address of the data associate
>  @var{handle}, or else @code{NULL} if there is none.
>  @end deftypefun
>  
> -The preceding functions can be combined with @code{lt_dlforeach} to
> -implement search and apply operations without the need for your
> -application to track the modules that have been loaded and unloaded:
> -
> [EMAIL PROTECTED]
> -int
> -my_dlinterface_callback (lt_dlhandle handle, void *key)
> [EMAIL PROTECTED]
> -  struct my_module_data *my_data;
> -
> -  my_data = lt_dlcaller_get_data (handle, (lt_dlinterface_id) key);
> -
> -  return process (my_data);
> [EMAIL PROTECTED]
> -
> -int
> -my_dlinterface_foreach (lt_dlinterface_id key)
> [EMAIL PROTECTED]
> -  lt_dlforeach (my_dlinterface_callback, (void *) key);
> [EMAIL PROTECTED]
> [EMAIL PROTECTED] example
> -
> [EMAIL PROTECTED] lt_dlhandle lt_dlhandle_first (@w{lt_dlinterface_id 
> @var{key}})
> -Normally, you can fetch each of the loaded module handles in turn with
> -successive calls to @samp{lt_dlhandle_next} as shown in the example
> -above.  In that example, the loop iterates over every libltdl loaded
> -module in your application, including the modules used by libltdl
> -itself!  This is useful from within a module loader for example.
> -
> [EMAIL PROTECTED]
> -Often, your application doesn't want to concern itself with modules
> -loaded by the libraries it uses, or for libltdl's internal use.  In
> -order to do that, you need to specify an interface validator callback:
> -
> [EMAIL PROTECTED]
> -/* @r{Return non-zero if} @var{handle} @r{doesn't conform to my iface.}  */
> -int
> -iface_validator_callback (lt_dlhandle handle, const char *id_string)
> [EMAIL PROTECTED]
> -    return (lt_sym (handle, "module_entry_point") != 0)
> [EMAIL PROTECTED]
> [EMAIL PROTECTED] example
> -
> [EMAIL PROTECTED]
> -When you register for an interface identification key with
> [EMAIL PROTECTED], you log the interface validator.  But
> -this time, when you start the iterator loop over the loaded module
> -handles, if you fetch the first handle with @samp{lt_dlhandle_first},
> -then that and all subsequent calls to @samp{lt_dlhandle_next} will
> -skip any loaded module handles that fail the registered interface
> -validator callback function:
> -
> [EMAIL PROTECTED]
> -/* @r{Register for an} interface_id @r{to identify ourselves to} libltdl.  */
> -interface_id = lt_dlinterface_register ("example", iface_validator_callback);
> -
> [EMAIL PROTECTED]
> -/* @r{Iterate over the modules related to my} interface_id.  */
> [EMAIL PROTECTED]
> -  lt_dlhandle handle;
> -  for (handle = lt_dlhandle_first (interface_id);
> -       handle;
> -       handle = lt_dlhandle_next (handle))
> -    @{
> -      @dots{}
> -    @}
> [EMAIL PROTECTED]
> [EMAIL PROTECTED] example
> [EMAIL PROTECTED] deftypefun
> -
>  Old versions of libltdl also provided a simpler, but similar, @sc{api}
>  based around @code{lt_dlcaller_id}.  Unfortunately, it had no
>  provision for detecting whether a module belonged to a particular
> Index: libtool--devo--1.0/libltdl/ltdl.c
> ===================================================================
> --- libtool--devo--1.0.orig/libltdl/ltdl.c
> +++ libtool--devo--1.0/libltdl/ltdl.c
> @@ -2113,91 +2113,69 @@ lt_dlgetinfo (lt_dlhandle handle)
>  }
>  
>  
> -/* Nasty semantics, necessary for reasonable backwards compatibility:
> -   Either iterate over the whole handle list starting with 
> lt_dlhandle_next(0),
> -   or else iterate over just the handles of modules that satisfy a given
> -   interface by getting the first element using lt_dlhandle_first(iface).  */
> -
> -static lt__interface_id *iterator = 0;
> -
> -lt_dlhandle
> -lt_dlhandle_first (lt_dlinterface_id iface)
> -{
> -  iterator = iface;
> -
> -  return handles;
> -}
> -
> -
>  lt_dlhandle
> -lt_dlhandle_next (lt_dlhandle place)
> +lt_dlhandle_iterate (lt_dlinterface_id iface, lt_dlhandle place)
>  {
>    lt__handle *handle = (lt__handle *) place;
> +  lt__interface_id *iterator = (lt__interface_id *) iface;
> +
> +  assert (iface); /* iface is a required argument */
>  
>    if (!handle)
> -    {
> -      /* old style iteration across all handles */
> -      iterator = 0;
> -      handle = (lt__handle *) handles;
> -    }
> -  else
> -    {
> -      /* otherwise start at the next handle after the passed one */
> -      handle = handle->next;
> -    }
> +    handle = (lt__handle *) handles;
>  
> -  /* advance until the interface check (if we have one) succeeds */
> -  while (handle && iterator && iterator->iface
> +  /* advance while the interface check fails */
> +  while (handle && iterator->iface
>        && ((*iterator->iface) (handle, iterator->id_string) != 0))
>      {
>        handle = handle->next;
>      }
>  
> -  if (!handle)
> -    {
> -      /* clear the iterator after the last handle */
> -      iterator = 0;
> -    }
> -
>    return (lt_dlhandle) handle;
>  }
>  
>  
>  lt_dlhandle
> -lt_dlhandle_find (const char *module_name)
> +lt_dlhandle_fetch (lt_dlinterface_id iface, const char *module_name)
>  {
> -  lt__handle *cur = (lt__handle *) handles;
> +  lt_dlhandle handle = 0;
> +
> +  assert (iface); /* iface is a required argument */
>  
> -  if (cur)
> +  while ((handle = lt_dlhandle_iterate (handle, iface)))
>      {
> -      do
> -     {
> -       if (cur->info.name && streq (cur->info.name, module_name))
> -         break;
> -     }
> -      while ((cur = cur->next));
> +      lt__handle *cur = (lt__handle *) handle;
> +      if (cur && cur->info.name && streq (cur->info.name, module_name))
> +     break;
>      }
>  
> -  return cur;
> +  return handle;
>  }
>  
> +
>  int
> -lt_dlforeach (int (*func) (lt_dlhandle handle, void *data), void *data)
> +lt_dlhandle_map (lt_dlinterface_id iface,
> +              int (*func) (lt_dlhandle handle, void *data), void *data)
>  {
> +  lt__interface_id *iterator = (lt__interface_id *) iface;
> +  lt__handle *cur = (lt__handle *) handles;
> +  lt__handle *tmp = 0;
> +  lt__handle *prev = 0;

Please remove tmp and prev, they are unused.

If you agree with my proposal to return the return value of func, you
can simplify this function by defining `errors' only inside the loop,
and returning immediately after calling `func' in the nonzero case.

>    int errors = 0;
> -  lt__handle *cur;
>  
> -  cur = (lt__handle *) handles;
> -  while (cur)
> -    {
> -      lt__handle *tmp = cur;
> +  assert (iface); /* iface is a required argument */
>  
> -      cur = cur->next;
> -      if ((*func) (tmp, data))
> +  while (cur && !errors)
> +    {
> +      /* advance while the interface check fails */
> +      while (cur && iterator->iface
> +          && ((*iterator->iface) (cur, iterator->id_string) != 0))
>       {
> -       ++errors;
> -       break;
> +       cur = cur->next;
>       }
> +
> +      if ((*func) (cur, data))
> +     ++errors;
>      }
>  
>    return errors;
> Index: libtool--devo--1.0/libltdl/ltdl.h
> ===================================================================
> --- libtool--devo--1.0.orig/libltdl/ltdl.h
> +++ libtool--devo--1.0/libltdl/ltdl.h
> @@ -126,10 +126,12 @@ typedef struct {
>  } lt_dlinfo;
>  
>  LT_SCOPE const lt_dlinfo *lt_dlgetinfo           (lt_dlhandle handle);
> -LT_SCOPE lt_dlhandle lt_dlhandle_first   (lt_dlinterface_id key);
> -LT_SCOPE lt_dlhandle lt_dlhandle_next    (lt_dlhandle place);
> -LT_SCOPE lt_dlhandle lt_dlhandle_find    (const char *module_name);
> -LT_SCOPE int         lt_dlforeach        (
> +
> +LT_SCOPE lt_dlhandle lt_dlhandle_iterate (lt_dlinterface_id iface,
> +                                          lt_dlhandle place);
> +LT_SCOPE lt_dlhandle lt_dlhandle_fetch   (lt_dlinterface_id iface,
> +                                          const char *module_name);
> +LT_SCOPE int         lt_dlhandle_map     (lt_dlinterface_id iface,
>                               int (*func) (lt_dlhandle handle, void *data),
>                               void *data);


Reply via email to