Re: Error reporting should be improved

[Peter Rosin]

Den 2009-12-30 01:25 skrev Bob Friesenhahn:
> This note in the Solaris manual page for dlerror() is interesting:
>
> NOTES
>      The messages returned by  dlerror() can reside in  a  static
>      buffer that is overwritten on each call to dlerror(). Appli-
>      cation code should not write to this buffer.  Programs want-
>      ing  to  preserve  an  error  message  should make their own
>      copies of that message.
>
> The implementation of lt__set_last_error  is:
>
>   static const char       *last_error     = 0;
>   const char *
>   lt__set_last_error (const char *errormsg)
>   {
>     return last_error = errormsg;
>   }
>
> so the current implementation is not reliable since the buffer pointed 
> to may be updated several times before it is used.  It is also not 
> thread safe.  Note that dlerror() may only be invoked once per error 
> since calling it a second time returns a null pointer.  The Linux manual 
> page recommends invoking dlerror() to clear any existing error message 
> before invoking dlsym() (which we are not doing) so a symbol which has a 
> valid value of zero will result in picking up some old error message.
>
> It is not clear how to make the interface completely thread safe, but it 
> is clear that the message needs to be copied immediately to a private 
> buffer before returning back to the caller in order to make sure that 
> the error returned is due to invoking a libltdl function.

Hi!

The thread safety thing can be ignored as libltdl isn't thread safe
and needs to be mutexed anyway. Care to test this patch?

Cheers,
Peter

        Make a copy of the returned dlerror string.

        * libltdl/lt_error.c (lt_dladderror): Don't allocate a new
        error number if the diagnostic string already exists.
        * libltdl/loaders/dlopen.c: Make use of the above and make
        sure the returned error string is copied as it might not
        survive another dlerror call.
        Report by Bob Friesenhahn.

diff --git a/libltdl/loaders/dlopen.c b/libltdl/loaders/dlopen.c
index 1d052b4..f67ca81 100644
--- a/libltdl/loaders/dlopen.c
+++ b/libltdl/loaders/dlopen.c
@@ -140,7 +140,7 @@ get_vtable (lt_user_data loader_data)
 #endif /* !RTLD_LOCAL */

 #if defined(HAVE_DLERROR)
-#  define DLERROR(arg) dlerror ()
+#  define DLERROR(arg) lt_dlseterror (lt_dladderror (dlerror ()))
 #else
 #  define DLERROR(arg) LT__STRERROR (arg)
 #endif
diff --git a/libltdl/lt_error.c b/libltdl/lt_error.c
index d7af36d..bfac8b5 100644
--- a/libltdl/lt_error.c
+++ b/libltdl/lt_error.c
@@ -51,7 +51,14 @@ lt_dladderror (const char *diagnostic)

   assert (diagnostic);

-  errindex = errorcount - LT_ERROR_MAX;
+  for (errindex = 0; errindex < errorcount - LT_ERROR_MAX; ++errindex)
+    {
+      if (!streq (user_error_strings[errindex], diagnostic))
+       continue;
+
+      return errindex + LT_ERROR_MAX;
+    }
+
   temp = REALLOC (const char *, user_error_strings, 1 + errindex);
   if (temp)
     {