Hi,
As I am sure many are aware, libltdl's error reporting is pretty dumb,
lt_dlerror() regularly reports things like "file not found" where the
actual problem might be something completely different, and a reasonable
error string may be readily available from dlerror().
When I looked at the manual, and read the description for lt_dlerror() I
saw "Return a human readable string describing the most recent error
that occurred from any of libltdl's functions. Return NULL if no errors
have occurred since initialization or since it was last called." and I
thought "Well, that's stupid - that's not what dlerror() does". So, I
went to see what POSIX says - it's also wrong -
http://www.opengroup.org/onlinepubs/9699919799/functions/dlerror.html, I
will file a bug with them later.
At least glibc and Mac OS X reset the dlerror() string to NULL every
time a dl*() api is called (I will check what other systems do in the
next few days). This is so programmers can do:
.....
dlsym(handle, "foo");
if ((err = dlerror()) != NULL) printf(stderr,"dlsym error: %s\n",err);
...
without dlerror() returning some other error that occurred long before
the call to dlsym().
Unfortunately, it looks like libltdl attempts to save error state across
calls to lt_*() functions, and generally does not do it well.
This patch resets the error every time a public ltdl function is called,
and when setting an error tests if it is already set and doesn't
overwrite it if it is. The fist error "wins".
The patch is fairly simple, and undoubtedly still imperfect, but test
results are good, one test required a small patch to stop segfaulting,
another should have unexpectedly passed, but doesn't, I think it is
another error in the test, but haven't looked too closely yet.
If something like this does get in, I'd prefer to not change the libltld
soname, I don't believe better error reporting will cause client
applications to break. I could be wrong, of course :)
Patch attached (or course, a final patch will have to change the manual
as well) - thoughts?
Peter
diff --git a/libltdl/libltdl/lt__private.h b/libltdl/libltdl/lt__private.h
index f4c4a3d..84f5622 100644
--- a/libltdl/libltdl/lt__private.h
+++ b/libltdl/libltdl/lt__private.h
@@ -138,7 +138,10 @@ struct lt__advise {
#define LT__GETERROR(lvalue) (lvalue) = lt__get_last_error()
#define LT__SETERRORSTR(errormsg) lt__set_last_error(errormsg)
-#define LT__SETERROR(errorcode) LT__SETERRORSTR(LT__STRERROR(errorcode))
+#define LT__SETERROR(errorcode) if (0 == lt__get_last_error()) \
+ LT__SETERRORSTR(LT__STRERROR(errorcode))
+#define LT__RESETERROR(x) lt__set_last_error(0)
+#define LT__FORCEERROR(errorstr) LT__SETERRORSTR(errorstr)
LT_SCOPE const char *lt__error_string (int errorcode);
LT_SCOPE const char *lt__get_last_error (void);
diff --git a/libltdl/loaders/preopen.c b/libltdl/loaders/preopen.c
index 7149287..58464ca 100644
--- a/libltdl/loaders/preopen.c
+++ b/libltdl/loaders/preopen.c
@@ -293,6 +293,7 @@ int
lt_dlpreload_default (const lt_dlsymlist *preloaded)
{
default_preloaded_symbols = preloaded;
+ LT__RESETERROR ();
return 0;
}
@@ -303,6 +304,7 @@ int
lt_dlpreload (const lt_dlsymlist *preloaded)
{
int errors = 0;
+ LT__RESETERROR ();
if (preloaded)
{
@@ -331,6 +333,7 @@ lt_dlpreload_open (const char *originator, lt_dlpreload_callback_func *func)
symlist_chain *list;
int errors = 0;
int found = 0;
+ LT__RESETERROR ();
/* For each symlist in the chain... */
for (list = preloaded_symlists; list; list = list->next)
diff --git a/libltdl/ltdl.c b/libltdl/ltdl.c
index e1b4e2f..098effa 100644
--- a/libltdl/ltdl.c
+++ b/libltdl/ltdl.c
@@ -216,6 +216,8 @@ int
lt_dlinit (void)
{
int errors = 0;
+ const char* saved_error = 0;
+ LT__RESETERROR ();
/* Initialize only at first call. */
if (++initialized == 1)
@@ -232,6 +234,7 @@ lt_dlinit (void)
/* Now open all the preloaded module loaders, so the application
can use _them_ to lt_dlopen its own modules. */
#ifdef HAVE_LIBDLLOADER
+ LT__GETERROR(saved_error);
if (!errors)
{
errors += lt_dlpreload (preloaded_symbols);
@@ -241,6 +244,7 @@ lt_dlinit (void)
{
errors += lt_dlpreload_open (LT_STR(LTDLOPEN), loader_init_callback);
}
+ LT__FORCEERROR(saved_error);
#endif /* HAVE_LIBDLLOADER */
}
@@ -258,6 +262,7 @@ lt_dlexit (void)
lt_dlloader *loader = 0;
lt_dlhandle handle = handles;
int errors = 0;
+ LT__RESETERROR ();
if (!initialized)
{
@@ -359,7 +364,6 @@ tryall_dlopen (lt_dlhandle *phandle, const char *filename,
lt_dladvise advise, const lt_dlvtable *vtable)
{
lt_dlhandle handle = handles;
- const char * saved_error = 0;
int errors = 0;
#ifdef LT_DEBUG_LOADERS
@@ -368,7 +372,6 @@ tryall_dlopen (lt_dlhandle *phandle, const char *filename,
vtable ? vtable->name : "(ALL)");
#endif
- LT__GETERROR (saved_error);
/* check whether the module was already opened */
for (;handle; handle = handle->next)
@@ -465,8 +468,6 @@ tryall_dlopen (lt_dlhandle *phandle, const char *filename,
handle->vtable = loader_vtable;
}
- LT__SETERRORSTR (saved_error);
-
done:
return errors;
}
@@ -844,8 +845,13 @@ load_deplibs (lt_dlhandle handle, char *deplibs)
if (strncmp(p, "-L", 2) == 0 || strncmp(p, "-R", 2) == 0)
{
char save = *end;
+ const char* saved_error = 0;
+ int r;
*end = 0; /* set a temporary string terminator */
- if (lt_dladdsearchdir(p+2))
+ LT__GETERROR(saved_error);
+ r = lt_dladdsearchdir(p+2);
+ LT__FORCEERROR(saved_error);
+ if (r)
{
goto cleanup;
}
@@ -1139,7 +1145,6 @@ static int
try_dlopen (lt_dlhandle *phandle, const char *filename, const char *ext,
lt_dladvise advise)
{
- const char * saved_error = 0;
char * archive_name = 0;
char * canonical = 0;
char * base_name = 0;
@@ -1158,7 +1163,6 @@ try_dlopen (lt_dlhandle *phandle, const char *filename, const char *ext,
ext ? ext : "(null)");
#endif
- LT__GETERROR (saved_error);
/* dlopen self? */
if (!filename)
@@ -1477,7 +1481,6 @@ try_dlopen (lt_dlhandle *phandle, const char *filename, const char *ext,
handles = *phandle;
}
- LT__SETERRORSTR (saved_error);
cleanup:
FREE (dir);
@@ -1537,6 +1540,7 @@ int
lt_dladvise_init (lt_dladvise *padvise)
{
lt_dladvise advise = (lt_dladvise) lt__zalloc (sizeof (struct lt__advise));
+ LT__RESETERROR ();
*padvise = advise;
return (advise ? 0 : 1);
}
@@ -1544,6 +1548,7 @@ lt_dladvise_init (lt_dladvise *padvise)
int
lt_dladvise_destroy (lt_dladvise *padvise)
{
+ LT__RESETERROR ();
if (padvise)
FREE(*padvise);
return 0;
@@ -1553,6 +1558,7 @@ int
lt_dladvise_ext (lt_dladvise *padvise)
{
assert (padvise && *padvise);
+ LT__RESETERROR ();
(*padvise)->try_ext = 1;
return 0;
}
@@ -1561,6 +1567,7 @@ int
lt_dladvise_resident (lt_dladvise *padvise)
{
assert (padvise && *padvise);
+ LT__RESETERROR ();
(*padvise)->is_resident = 1;
return 0;
}
@@ -1569,6 +1576,7 @@ int
lt_dladvise_local (lt_dladvise *padvise)
{
assert (padvise && *padvise);
+ LT__RESETERROR ();
(*padvise)->is_symlocal = 1;
return 0;
}
@@ -1577,6 +1585,7 @@ int
lt_dladvise_global (lt_dladvise *padvise)
{
assert (padvise && *padvise);
+ LT__RESETERROR ();
(*padvise)->is_symglobal = 1;
return 0;
}
@@ -1585,6 +1594,7 @@ int
lt_dladvise_preload (lt_dladvise *padvise)
{
assert (padvise && *padvise);
+ LT__RESETERROR ();
(*padvise)->try_preload_only = 1;
return 0;
}
@@ -1606,11 +1616,14 @@ lt_dlopenext (const char *filename)
{
lt_dlhandle handle = 0;
lt_dladvise advise;
+ const char* saved_error = 0;
if (!lt_dladvise_init (&advise) && !lt_dladvise_ext (&advise))
handle = lt_dlopenadvise (filename, advise);
+ LT__GETERROR(saved_error);
lt_dladvise_destroy (&advise);
+ LT__FORCEERROR(saved_error);
return handle;
}
@@ -1620,9 +1633,7 @@ lt_dlopenadvise (const char *filename, lt_dladvise advise)
{
lt_dlhandle handle = 0;
int errors = 0;
- const char * saved_error = 0;
-
- LT__GETERROR (saved_error);
+ LT__RESETERROR ();
/* Can't have symbols hidden and visible at the same time! */
if (advise && advise->is_symlocal && advise->is_symglobal)
@@ -1659,7 +1670,6 @@ lt_dlopenadvise (const char *filename, lt_dladvise advise)
#if defined(LT_MODULE_EXT)
/* Try appending SHLIB_EXT. */
- LT__SETERRORSTR (saved_error);
errors = try_dlopen (&handle, filename, shlib_ext, advise);
/* As before, if the file was found but loading failed, return now
@@ -1866,6 +1876,7 @@ lt_dlforeachfile (const char *search_path,
{
int is_done = 0;
file_worker_func **fpptr = &func;
+ LT__RESETERROR ();
if (search_path)
{
@@ -1909,6 +1920,7 @@ lt_dlclose (lt_dlhandle handle)
{
lt_dlhandle cur, last;
int errors = 0;
+ LT__RESETERROR ();
/* check whether the handle is valid */
last = cur = handles;
@@ -1978,6 +1990,8 @@ lt_dlsym (lt_dlhandle place, const char *symbol)
lt_user_data data;
lt_dlhandle handle;
+ LT__RESETERROR ();
+
if (!place)
{
LT__SETERROR (INVALID_HANDLE);
@@ -2012,9 +2026,6 @@ lt_dlsym (lt_dlhandle place, const char *symbol)
data = handle->vtable->dlloader_data;
if (handle->info.name)
{
- const char *saved_error;
-
- LT__GETERROR (saved_error);
/* this is a libtool module */
if (handle->vtable->sym_prefix)
@@ -2040,7 +2051,6 @@ lt_dlsym (lt_dlhandle place, const char *symbol)
}
return address;
}
- LT__SETERRORSTR (saved_error);
}
/* otherwise try "symbol" */
@@ -2054,6 +2064,10 @@ lt_dlsym (lt_dlhandle place, const char *symbol)
strcpy(sym, symbol);
}
+ /* Reset the dlerror() string, so that errors returned are for the
+ * symbol without the foo_LTX_ prefix */
+ LT__RESETERROR ();
+
address = handle->vtable->find_sym (data, handle->module, sym);
if (sym != lsym)
{
@@ -2148,6 +2162,8 @@ lt_dladdsearchdir (const char *search_dir)
{
int errors = 0;
+ LT__RESETERROR ();
+
if (search_dir && *search_dir)
{
if (lt_dlpath_insertdir (&user_search_path, 0, search_dir) != 0)
@@ -2161,6 +2177,7 @@ int
lt_dlinsertsearchdir (const char *before, const char *search_dir)
{
int errors = 0;
+ LT__RESETERROR ();
if (before)
{
@@ -2188,6 +2205,7 @@ int
lt_dlsetsearchpath (const char *search_path)
{
int errors = 0;
+ LT__RESETERROR ();
FREE (user_search_path);
@@ -2206,6 +2224,7 @@ const char *
lt_dlgetsearchpath (void)
{
const char *saved_path;
+ LT__RESETERROR ();
saved_path = user_search_path;
diff --git a/tests/lt_dlopenext.at b/tests/lt_dlopenext.at
index 26a2b24..66c6c0c 100644
--- a/tests/lt_dlopenext.at
+++ b/tests/lt_dlopenext.at
@@ -120,7 +120,8 @@ main (int argc, const char *argv[])
errormsg = dlerrordup (errormsg);
if (errormsg != NULL)
{
- errors = lt_dlclose (module);
+ errors = 1;
+ errors += lt_dlclose (module);
module = NULL;
}
}
_______________________________________________
http://lists.gnu.org/mailman/listinfo/libtool
