Hi Ralf,
Den 2010-01-24 11:59 skrev Ralf Wildenhues:
Hi Peter,
* Peter Rosin wrote on Fri, Jan 22, 2010 at 09:53:40AM CET:
So, I set out to update it, see attached patch.
When going through this, I found it strange that
lt_dlloader_add didn't call vtable->dlloader_init. Isn't
that desirable? I'll send an additional patch to show what
I mean...
That one's already been shot down. ;->
Yes, I thought of that myself after sending the mail, but figured
I'd see if anyone else would catch it... ;-]
The comment about us needing testsuite exposure for this kind of code is
still very much valid though. We should try and see how far we can get
with the current documented sample code.
Not very far at all unless you propose to add some kind of compatibility
layer...
You'd have to make aliases so that "dlopen" == "lt_dlopen" *and*
"dlopen" == "lt_loadlibrary" (since "dlopen" is used for "host dependent
module loading"). How's that going to work on Cygwin that has both?
But ok, that's fairly trivial, and I suppose "dlopen" should get you
the lt_dlopen loader on Cygwin, but that overload of "dlopen" is
not supported by current ltdl.
Also, lt_dlloader_add (and others) have completely incompatible
signatures. There's no way around that. The documentation is just
too damn broken to have any relevance at all when there is
"competition" for some aspect of the api, if you ask me.
- the current code requires to pass in a structure allocated on the heap
(i.e., we keep a pointer to it); I'm inclined to want to fix ltdl by
copying the (documented part of the) input rather than change the
documented API though. Never mind the small memleak that ensues
(this should still have a NEWS entry however).
Making provisions for the neglected api documentation that introduce
an unaviodaable(?) memory leak is not the way to go, if you ask me.
- documenting the contents of a structure is bad bad bad. Since we
already do it, it is cast in stone and cannot be changed (neither
amended, because even that will cause old compiled code to break),
In this case I think it can because anyone trying to read the docs
to get something done will quickly see that the docs is just plain
broken. They would have to consult the code to get anything done.
Any user relying on docs (or anything really) that is so clearly
broken is just stupid.
Anyway, I did some data mining...
Commit 5451db06bd5391fa674846acba23b1a1ab778bc9 was "Okay to commit?"-ed
here:
http://lists.gnu.org/archive/html/libtool-patches/2004-07/msg00026.html
and it was apparently commited by the 72 hour rule, since I find
no ack/nack.
Anyway, that commit has no matching change in docs which is the origin
of the problem. So, the docs matches what the 1.5 series is doing, but
not the reality since 1.9b.
Here's a gentoo bug report for pulseaudio that seems releated:
http://bugs.gentoo.org/show_bug.cgi?id=281342
Here's what they had to do in pulseaudio when they moved to libtool 2.2:
http://git.0pointer.de/?p=pulseaudio.git;a=commitdiff;h=9ad7bb6188a6e3f01ae105473ad822ab81246627
But they didn't get it right from the beginning so this is a fixup:
http://git.0pointer.de/?p=pulseaudio.git;a=commitdiff;h=b8fe1b683e9d373bb3475b30099593f00eaf4cff
They had a few other changes in between as well...
See, people notice that ltdl has broken their code, and they somehow
pick up how to do it the new way w/o matching docs. They struggle a
bit though, and are not getting it right from the start, which is why
I think it would be better to just document the current api and be
done with it...
Also notice how they did use "dlopen" and how that was portable at
one point, but how they are now non-portable when they simply
converted that to "lt_dlopen".
But I don't get how to change the docs so that they suite you,
perhaps it's easiest if you did this part yourself?
- long deftype lines should be wrapped with @<newline>
I don't understand how to wrap the lines from that, please give
an example (or do it yourself, since I'm passing the buck anyway).
- as argued in the other thread, lt_dlloader_init should not be
documented nor used.
I have zapped that paragraph.
- typo: responible
*snip*
That part and the perror removals look fairly ok to me, with
s/Win32/Windows/.
That's very little to fork out and commit for fairly limited value.
I changed "system dynamic loader" to "POSIX dynamic loader" for
"lt_dlopen" though...
I'll just attach my current patch and you can take it from there. I'll
volonteer for the testcase though, coming soonish...
Cheers,
Peter
--
They are in the crowd with the answer before the question.
> Why do you dislike Jeopardy?
diff --git a/doc/libtool.texi b/doc/libtool.texi
index 482e635..8200b85 100644
--- a/doc/libtool.texi
+++ b/doc/libtool.texi
@@ -4282,16 +4282,24 @@ can be recognised by @code{lt_dlloader_find} and
removed with
already in use by libltdl's builtin loaders:
@table @asis
-...@item "dlopen"
-The system dynamic library loader, if one exists.
-...@item "dld"
+...@item "lt_dlopen"
+The POSIX dynamic library loader, if one exists.
+...@item "lt_dld_link"
The @sc{gnu} dld loader, if @file{libdld} was installed when libltdl was
built.
-...@item "dlpreload"
-The loader for @code{lt_dlopen}ing of preloaded static modules.
+...@item "lt_preopen"
+The loader for @code{lt_dlopen}ing of preopened static modules.
+...@item "lt_dyld"
+The Darwin / OS X dynamic library loader.
+...@item "lt_load_add_on"
+The BeOS dynamic library loader.
+...@item "lt_loadlibrary"
+The Windows dynamic library loader.
+...@item "lt_shl_load"
+The HP/UX dynamic library loader.
@end table
-The prefix "dl" is reserved for loaders supplied with future versions of
+The prefix "lt_" is reserved for loaders supplied with future versions of
libltdl, so you should not use that for your own loader names.
@noindent
@@ -4311,7 +4319,11 @@ level types.
@code{lt_user_data} is used for specifying loader instance data.
@end deftp
-...@deftypefn {Type} {struct} lt_user_dlloader @{...@w{const char
*...@var{sym_prefix};} @w{lt_module_open *...@var{module_open};}
@w{lt_module_close *...@var{module_close};} @w{lt_find_sym *...@var{find_sym};}
@w{lt_dlloader_exit *...@var{dlloader_exit};} @}
+...@deftp {Type} lt_dlloader_priority
+...@code{lt_dlloader_priority} is used for specifying the loader priority.
+...@end deftp
+
+...@deftypefn {Type} {struct} lt_dlvtable @{...@w{const char *...@var{name};}
@w{const char *...@var{sym_prefix};} @w{lt_module_open *...@var{module_open};}
@w{lt_module_close *...@var{module_close};} @w{lt_find_sym *...@var{find_sym};}
@w{lt_dlloader_init *...@var{dlloader_init};} @w{lt_dlloader_exit
*...@var{dlloader_exit};} @w{lt_user_data @var{dlloader_data};}
@w{lt_dlloader_priority @var{priority};} @}
If you want to define a new way to open dynamic modules, and have the
@code{lt_dlopen} @sc{api} use it, you need to instantiate one of these
structures and pass it to @code{lt_dlloader_add}. You can pass whatever
@@ -4361,22 +4373,35 @@ For example:
int
register_myloader (void)
@{
- lt_user_dlloader dlloader;
+ lt_dlvtable *myloader;
+
+ myloader = malloc (sizeof (*myloader));
+ if (!myloader)
+ return MEMORY_ERROR;
/* User modules are responsible for their own initialisation. */
if (myloader_init () != 0)
- return MYLOADER_INIT_ERROR;
+ @{
+ free (myloader);
+ return MYLOADER_INIT_ERROR;
+ @}
- dlloader.sym_prefix = NULL;
- dlloader.module_open = myloader_open;
- dlloader.module_close = myloader_close;
- dlloader.find_sym = myloader_find_sym;
- dlloader.dlloader_exit = myloader_exit;
- dlloader.dlloader_data = (lt_user_data)myloader_function;
+ myloader->name = "myloader";
+ myloader->sym_prefix = NULL;
+ myloader->module_open = myloader_open;
+ myloader->module_close = myloader_close;
+ myloader->find_sym = myloader_find_sym;
+ myloader->dlloader_exit = myloader_exit;
+ myloader->dlloader_data = (lt_user_data)myloader_function;
+ myloader->priority = LT_DLLOADER_PREPEND;
/* Add my loader as the default module loader. */
- if (lt_dlloader_add (lt_dlloader_next (NULL), &dlloader, "myloader") != 0)
- return ERROR;
+ if (lt_dlloader_add (myloader) != 0)
+ @{
+ myloader_exit (myloader->dlloader_data);
+ free (myloader);
+ return ERROR;
+ @}
return OK;
@}
@@ -4395,55 +4420,61 @@ during the initialisation phase.
libltdl provides the following functions for writing your own module
loaders:
-...@deftypefun int lt_dlloader_add (@w{lt_dlloader *...@var{place},}
@w{lt_user_dlloader *...@var{dlloader},} @w{const char *...@var{loader_name}})
+...@deftypefun int lt_dlloader_add (@w{lt_dlvtable *...@var{dlloader}})
Add a new module loader to the list of all loaders, either as the
-last loader (if @var{place} is @code{NULL}), else immediately before the
-loader passed as @var{place}. @var{loader_name} will be returned by
-...@code{lt_dlloader_name} if it is subsequently passed a newly
-registered loader. These @var{loader_name}s must be unique, or
-...@code{lt_dlloader_remove} and @code{lt_dlloader_find} cannot
-work. Returns 0 for success.
+last loader (if @var{priority} in @var{dlloader} is set to
+...@code{lt_dlloader_append}), else the first loader (if it is set
+to @code{LT_DLLOADER_PREPEND}). The @var{loader_name} given in
+...@var{dlloader} will be returned by @code{lt_dlloader_name} if it
+is subsequently passed a newly registered loader. These
+...@var{loader_name}s must be unique, or @code{lt_dlloader_remove}
+and @code{lt_dlloader_find} cannot work. Returns 0 for success.
@example
/* Make myloader be the last one. */
-if (lt_dlloader_add (NULL, myloader) != 0)
- perror (lt_dlerror ());
+myloader.priority = LT_DLLOADER_APPEND;
+if (lt_dlloader_add (&myloader) != 0)
+ fprintf (stderr, "lt_dlloader_add failed: %s\n", lt_dlerror ());
@end example
@end deftypefun
-...@deftypefun int lt_dlloader_remove (@w{const char *...@var{loader_name}})
+...@deftypefun {lt_dlvtable *}lt_dlloader_remove (@w{const char
*...@var{loader_name}})
Remove the loader identified by the unique name, @var{loader_name}.
Before this can succeed, all modules opened by the named loader must
-have been closed. Returns 0 for success, otherwise an error message can
-be obtained from @code{lt_dlerror}.
+have been closed. Returns the vtable of the removed loader for success,
+otherwise @code{NULL} in which case an error message can be obtained
+from @code{lt_dlerror}. The caller is responsible for @code{free}ing the
+returned @var{vtable}, if that is needed.
@example
/* Remove myloader. */
-if (lt_dlloader_remove ("myloader") != 0)
- perror (lt_dlerror ());
+lt_dlvtable *myloader = lt_dlloader_remove ("myloader");
+if (myloader)
+ free (myloader);
+else
+ fprintf (stderr, "lt_dlloader_remove failed: %s\n", lt_dlerror ());
+myloader = NULL;
@end example
@end deftypefun
-...@deftypefun {lt_dlloader *}lt_dlloader_next (@w{lt_dlloader
*...@var{place}})
-Iterate over the module loaders, returning the first loader if @var{place} is
-...@code{null}, and the next one on subsequent calls. The handle is for use
with
-...@code{lt_dlloader_add}.
+...@deftypefun lt_dlloader lt_dlloader_next (@w{const lt_dlloader
@var{loader}})
+Iterate over the module loaders, returning the first loader if @var{loader} is
+...@code{null}, and the next one on subsequent calls, or @code{NULL} if at the
+end of the list. The handle is for use with @code{lt_dlloader_get}.
@example
-/* Make myloader be the first one. */
-if (lt_dlloader_add (lt_dlloader_next (NULL), myloader) != 0)
- return ERROR;
+/* Print the name of the first loader. */
+puts (lt_dlloader_get (lt_dlloader_next (NULL))->name);
@end example
@end deftypefun
-...@deftypefun {lt_dlloader *}lt_dlloader_find (@w{const char
*...@var{loader_name}})
+...@deftypefun {const lt_dlvtable *}lt_dlloader_find (@w{const char
*...@var{loader_name}})
Return the first loader with a matching @var{loader_name} identifier, or else
@code{NULL}, if the identifier is not found.
The identifiers that may be used by libltdl itself, if the host
-architecture supports them are @dfn{dlop...@footnote{this is used for
-the host dependent module loading @sc{api} -- @code{shl_load} and
-...@code{loadlibrary} for example}, @dfn{dld} and @dfn{dlpreload}.
+architecture supports them are e.g. @dfn{lt_dlopen}, @dfn{lt_dld_link},
+...@dfn{lt_preopen}, @dfn{lt_dyld} and @dfn{lt_loadlibrary}.
@example
/* Add a user loader as the next module loader to be tried if
@@ -4453,20 +4484,13 @@ if (lt_dlloader_add (lt_dlloader_find ("dlopen"),
myloader) != 0)
@end example
@end deftypefun
-...@deftypefun {const char *}lt_dlloader_name (@w{lt_dlloader *...@var{place}})
-Return the identifying name of @var{PLACE}, as obtained from
-...@code{lt_dlloader_next} or @code{lt_dlloader_find}. If this function fails,
-it will return @code{NULL} and set an error for retrieval with
+...@deftypefun {const lt_dlvtable *}lt_dlloader_get (@w{lt_dlloader
@var{loader}})
+This function returns the @var{vtable} associated with the @var{loader},
+as obtained from @code{lt_dlloader_next}. If this function fails, it
+will return @code{NULL} and set an error for retrieval with
@code{lt_dlerror}.
@end deftypefun
-...@deftypefun {lt_user_data *}lt_dlloader_data (@w{lt_dlloader
*...@var{place}})
-Return the address of the @code{dlloader_data} of @var{PLACE}, as
-obtained from @code{lt_dlloader_next} or @code{lt_dlloader_find}. If
-this function fails, it will return @code{NULL} and set an error for
-retrieval with @code{lt_dlerror}.
-...@end deftypefun
-
@subsection Error handling within user module loaders
@deftypefun int lt_dladderror (@w{const char *...@var{diagnostic}})
@@ -4480,7 +4504,7 @@ If the allocation of an identifier fails, this function
returns -1.
@example
int myerror = lt_dladderror ("Doh!");
if (myerror < 0)
- perror (lt_dlerror ());
+ fprintf (stderr, "lt_dladderror failed: %s\n", lt_dlerror ());
@end example
@end deftypefun
@@ -4493,7 +4517,7 @@ interface. All of the standard errors used by libltdl
are declared in
@example
if (lt_dlseterror (LTDL_ERROR_NO_MEMORY) != 0)
- perror (lt_dlerror ());
+ fprintf (stderr, "lt_dlseterror failed: %s\n", lt_dlerror ());
@end example
@end deftypefun