[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 370] Implement lt_dlopening of only preloaded modules.
From: |
Ralf Wildenhues |
Subject: |
Re: [PATCH 370] Implement lt_dlopening of only preloaded modules. |
Date: |
Tue, 6 May 2008 20:07:14 +0200 |
User-agent: |
Mutt/1.5.17+20080114 (2008-01-14) |
Hi Gary,
a quick nit review (I haven't looked thoroughly yet):
* Gary V. Vaughan wrote on Tue, May 06, 2008 at 07:24:46PM CEST:
>
> This makes it possible to build a libltdl client with some preloaded
> modules, and limit lt_dlopenadvise to open only the preloaded modules.
> --- a/NEWS
> +++ b/NEWS
> @@ -2,9 +2,10 @@ NEWS - list of user-visible changes between releases of GNU
> Libtool
>
> New in 2.2.??: 2008-06-??: git version 2.2.5a, Libtool team:
>
> -* Bug fixes:
> +* New features:
>
> - - None yet
No need to remove the Bug fixes section, there will certainly be entries
later, no? ;-)
> --- a/doc/libtool.texi
> +++ b/doc/libtool.texi
> @@ -3816,6 +3816,13 @@ On failure, @code{lt_dladvise_resident} returns
> non-zero and sets an error
> message that can be retrieved with @code{lt_dlerror}.
> @end deftypefun
>
> address@hidden int lt_dladvise_preload (lt_dladvise address@hidden)
> +Set the @code{preload} hint on @var{advise}. Passing an @var{advise}
> +parameter to @code{lt_dlopenadvise} with this hint set causes it to
> +load only preloaded modules, so that if a suitable preleaded module is
s/preleaded/preloaded/
> +not found, @code{lt_dlopenadvise} will return @code{NULL}.
> address@hidden deftypefun
> +
> --- a/libltdl/libltdl/lt__private.h
> +++ b/libltdl/libltdl/lt__private.h
> @@ -126,6 +126,7 @@ struct lt__advise {
> subsequently loaded modules. */
> unsigned int is_symlocal:1; /* module symbols are only available
> locally. */
> + unsigned int is_preload:1; /* only preloaded modules will be
> tried. */
s/is_preload/try_preload_only/ ?
> --- a/libltdl/ltdl.c
> +++ b/libltdl/ltdl.c
> @@ -1257,27 +1257,40 @@ try_dlopen (lt_dlhandle *phandle, const char
> *filename, const char *ext,
>
> if (vtable)
> {
> + char *archive_name = MALLOC (char, LT_STRLEN (name) + 3);
> *phandle = (lt_dlhandle) lt__zalloc (sizeof (struct lt__handle));
>
> - if (*phandle == NULL)
> + if ((*phandle == NULL) || (archive_name == NULL))
> {
> ++errors;
> goto cleanup;
> }
> newhandle = *phandle;
>
> - if (tryall_dlopen (&newhandle, attempt, advise, vtable) == 0)
> + /* Preloaded modules are always named according to their old
> + archive name. */
> + sprintf (archive_name, "%s.a", name);
> +
> + if (tryall_dlopen (&newhandle, archive_name, advise, vtable) == 0)
> {
> goto register_handle;
Leaking archive_name here.
> }
>
> /* If we're still here, there was no matching preloaded module,
> so put things back as we found them, and continue searching. */
> + FREE (archive_name);
> FREE (*phandle);
> newhandle = NULL;
> }
> }
>
> --- a/libltdl/m4/ltdl.m4
> +++ b/libltdl/m4/ltdl.m4
> @@ -264,7 +264,7 @@ if test "x$with_included_ltdl" != xyes; then
> # decide whether there is a useful installed version we can use.
> AC_CHECK_HEADER([ltdl.h],
> [AC_CHECK_DECL([lt_dlinterface_register],
> - [AC_CHECK_LIB([ltdl], [lt_dlinterface_register],
> + [AC_CHECK_LIB([ltdl], [lt_dladvise_preload],
> [with_included_ltdl=no],
> [with_included_ltdl=yes])],
> [with_included_ltdl=yes],
I view this as a separate change. Not sure whether we should have a
policy to always require the newest available feature or not, in an
installed ltdl.
> --- a/tests/lt_dladvise.at
> +++ b/tests/lt_dladvise.at
AFAICS the test doesn't ensure that, if the platform supports
non-preloaded module, lt_dladvise_preload causes trying to load them to
fail.
Cheers,
Ralf