Hi Ralf,
Den 2010-01-20 22:15 skrev Ralf Wildenhues:
* Peter Rosin wrote on Sun, Jan 17, 2010 at 09:19:27PM CET:
The \r\n at the end of the messages should probably be removed.
Sounds like a good idea to me.
I have since added that to the loadlibraryerror function.
And I did
manage to write a test that exposes this new functionallity - as you
requested - by unloading all modules except the loadlibrary loader (and
the preopen loader). The test is attached.
Good. Thanks. I can't find obvious issues with the test, and it should
skip on all unixy systems (due to loadlibrary loader not being present).
I have checked that it indeed skips on linux due to no loadlibrary loader.
But, I have not succeeded in generating any other errors than the above
two. I have tried to remove execute bits on the plugin and a dependent
library, and I have tried removing a dependent library. But real problems
usually manifests themselves in weird ways and the correct error message
may be crucial in some unforseen case. Is it worth it?
We can cross that bridge when we get to it, i.e., when we have a problem
report with a setup that provoke such an error. The test case is good
because it ensures several code paths are covered in the source. If it
works on MiNGW (I gather from above that you've already tested Cygwin
and Wine) then feel free to commit both patch and test.
I did some minor tweaks to the test after going through the following
systems, listing the test result after the tweaks:
MSYS/MinGW ok
Wine/MinGW ok
Wine/MinGW --disable-static skip (empty $dlname in the .la file)
Cygwin/gcc ok
linux/gcc skip (no loadlibrary loader)
So, I'm pushing as attached. Thanks for the review!
Cheers,
Peter
2010-01-20 Peter Rosin <p...@lysator.liu.se>
Report proper errors from the loadlibrary loader.
* libltdl/loaders/loadlibrary.c (loadlibraryerror): New
helper function that returns the latest Windows error as a
string, or the provided default string on failure to do so.
(LOADLIB_SETERROR): New macro that wraps previous to make it
easy to use.
(vm_open, vm_close, vm_sym): Make use of previous.
(LOCALFREE): New macro to help free the Windows error string.
(vl_exit): Make use of previous.
* tests/loadlibarry.at: New file, new test that makes sure
the loadlibrary loader reports non-standard error messages.
* Makefile.am (TESTSUITE_AT): Add above test.
Signed-off-by: Peter Rosin <p...@lysator.liu.se>
--
They are in the crowd with the answer before the question.
> Why do you dislike Jeopardy?
diff --git a/Makefile.am b/Makefile.am
index 8d6682b..40a0138 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -485,6 +485,7 @@ TESTSUITE_AT = tests/testsuite.at \
tests/lt_dlopen_a.at \
tests/lt_dlopenext.at \
tests/ltdl-api.at \
+ tests/loadlibrary.at \
tests/lalib-syntax.at \
tests/resident.at \
tests/slist.at \
diff --git a/libltdl/loaders/loadlibrary.c b/libltdl/loaders/loadlibrary.c
index 40435fe..139851a 100644
--- a/libltdl/loaders/loadlibrary.c
+++ b/libltdl/loaders/loadlibrary.c
@@ -98,12 +98,19 @@ get_vtable (lt_user_data loader_data)
#include <windows.h>
+#define LOCALFREE(mem) LT_STMT_START { \
+ if (mem) { LocalFree ((void *)mem); mem = NULL; } } LT_STMT_END
+#define LOADLIB__SETERROR(errmsg) LT__SETERRORSTR (loadlibraryerror (errmsg))
+#define LOADLIB_SETERROR(errcode) LOADLIB__SETERROR (LT__STRERROR (errcode))
+
+static const char *loadlibraryerror (const char *default_errmsg);
static UINT WINAPI wrap_geterrormode (void);
static UINT WINAPI fallback_geterrormode (void);
typedef UINT (WINAPI geterrormode_type) (void);
static geterrormode_type *geterrormode = wrap_geterrormode;
+static char *error_message = 0;
/* A function called through the vtable when this loader is no
@@ -112,6 +119,7 @@ static int
vl_exit (lt_user_data LT__UNUSED loader_data)
{
vtable = NULL;
+ LOCALFREE (error_message);
return 0;
}
@@ -213,7 +221,9 @@ vm_open (lt_user_data LT__UNUSED loader_data, const char
*filename,
}
}
- if (cur || !module)
+ if (!module)
+ LOADLIB_SETERROR (CANNOT_OPEN);
+ else if (cur)
{
LT__SETERROR (CANNOT_OPEN);
module = 0;
@@ -231,9 +241,9 @@ vm_close (lt_user_data LT__UNUSED loader_data, lt_module
module)
{
int errors = 0;
- if (FreeLibrary((HMODULE) module) == 0)
+ if (FreeLibrary ((HMODULE) module) == 0)
{
- LT__SETERROR (CANNOT_CLOSE);
+ LOADLIB_SETERROR (CANNOT_CLOSE);
++errors;
}
@@ -250,7 +260,7 @@ vm_sym (lt_user_data LT__UNUSED loader_data, lt_module
module, const char *name)
if (!address)
{
- LT__SETERROR (SYMBOL_NOT_FOUND);
+ LOADLIB_SETERROR (SYMBOL_NOT_FOUND);
}
return address;
@@ -261,6 +271,33 @@ vm_sym (lt_user_data LT__UNUSED loader_data, lt_module
module, const char *name)
/* --- HELPER FUNCTIONS --- */
+/* Return the windows error message, or the passed in error message on
+ failure. */
+static const char *
+loadlibraryerror (const char *default_errmsg)
+{
+ size_t len;
+ LOCALFREE (error_message);
+
+ FormatMessageA (FORMAT_MESSAGE_ALLOCATE_BUFFER |
+ FORMAT_MESSAGE_FROM_SYSTEM |
+ FORMAT_MESSAGE_IGNORE_INSERTS,
+ NULL,
+ GetLastError (),
+ 0,
+ (char *) &error_message,
+ 0, NULL);
+
+ /* Remove trailing CRNL */
+ len = LT_STRLEN (error_message);
+ if (len && error_message[len - 1] == '\n')
+ error_message[--len] = LT_EOS_CHAR;
+ if (len && error_message[len - 1] == '\r')
+ error_message[--len] = LT_EOS_CHAR;
+
+ return len ? error_message : default_errmsg;
+}
+
/* A function called through the geterrormode variable which checks
if the system supports GetErrorMode and arranges for it or a
fallback implementation to be called directly in the future. The
diff --git a/tests/loadlibrary.at b/tests/loadlibrary.at
new file mode 100644
index 0000000..8315a5d
--- /dev/null
+++ b/tests/loadlibrary.at
@@ -0,0 +1,251 @@
+# loadlibrary.at -- test loadlibrary functionality -*- Autotest -*-
+#
+# Copyright (C) 2010 Free Software Foundation, Inc.
+# This file is part of GNU Libtool.
+#
+# GNU Libtool is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# GNU Libtool is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GNU Libtool; see the file COPYING. If not, a copy
+# can be downloaded from http://www.gnu.org/licenses/gpl.html,
+# or obtained by writing to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+####
+
+AT_SETUP([loadlibrary error messages])
+AT_KEYWORDS([libltdl])
+
+eval "`$LIBTOOL --config | $EGREP '^(objdir)='`"
+
+AT_DATA([main.c],
+[[#include <ltdl.h>
+#include <stdio.h>
+
+static int
+standard_error_message(const char *error)
+{
+ int error_number;
+
+ for (error_number = 0; error_number < LT_ERROR_MAX; ++error_number)
+ {
+ lt_dlseterror (error_number);
+ if (error == lt_dlerror ())
+ {
+ return 1;
+ }
+ }
+
+ lt_dlseterror (LT_ERROR_UNKNOWN);
+ return 0;
+}
+
+int
+main (int argc, char* argv[])
+{
+ int err = 0;
+ lt_dlhandle module = NULL;
+ const lt_dlvtable *loadlibrary;
+ const lt_dlvtable *preopen;
+ lt_dlloader *loader = NULL;
+ lt_dlloader *next;
+ const lt_dlvtable *vtable;
+ void *symbol;
+ const char *error;
+
+ if (argc < 2)
+ {
+ fprintf (stderr, "usage: %s plugin [symbol]\n", argv[0]);
+ return 1;
+ }
+
+ lt_dlinit ();
+
+ loadlibrary = lt_dlloader_find ("lt_loadlibrary");
+ if (!loadlibrary)
+ {
+ /* Skip if the loadlibrary loader isn't supported */
+ printf ("loadlibrary loader not found\n");
+ err = 77;
+ goto cleanup;
+ }
+
+ preopen = lt_dlloader_find ("lt_preopen");
+ if (!loadlibrary)
+ {
+ printf ("preopen loader not found\n");
+ err = 2;
+ goto cleanup;
+ }
+
+ /* Remove all loaders except the preopen and loadlibrary loaders. */
+ while (next = lt_dlloader_next (loader))
+ {
+ if (lt_dlloader_get (next) == loadlibrary)
+ {
+ loader = next;
+ continue;
+ }
+
+ if (lt_dlloader_get (next) == preopen)
+ {
+ loader = next;
+ continue;
+ }
+
+ lt_dlloader_remove (lt_dlloader_get (next)->name);
+ }
+
+ module = lt_dlopen (argv[1]);
+ error = lt_dlerror ();
+
+ if (module)
+ {
+ printf ("lt_dlopen: success!\n");
+
+ if (argc == 2)
+ {
+ /* But failure was the desired result... */
+ printf ("expected failure\n");
+ err = 2;
+ goto cleanup;
+ }
+ }
+ else if (argc > 2)
+ {
+ /* Didn't expect failure... */
+ printf ("lt_dlopen: failure: %s\n", error);
+ err = 2;
+ goto cleanup;
+ }
+ else if (standard_error_message (error))
+ {
+ /* Expected custom error message... */
+ printf ("lt_dlopen: standard error (bad): %s\n", error);
+ err = 1;
+ goto cleanup;
+ }
+ else
+ {
+ printf ("lt_dlopen: custom error (good): %s\n", error);
+ goto cleanup;
+ }
+
+ symbol = lt_dlsym (module, argv[2]);
+ error = lt_dlerror ();
+
+ if (symbol)
+ {
+ printf ("lt_dlsym: success!\n");
+ }
+ else if (standard_error_message (error))
+ {
+ /* Expected custom failure message... */
+ printf ("lt_dlsym: standard error (bad): %s\n", error);
+ err = 1;
+ }
+ else
+ {
+ printf ("lt_dlsym: custom error (good): %s\n", error);
+ }
+
+cleanup:
+ if (module)
+ {
+ lt_dlclose (module);
+ }
+ lt_dlexit ();
+ return err;
+}
+]])
+
+AT_DATA([foomod.c],
+[[
+int foosym (void);
+int
+foosym (void)
+{
+ return 0;
+}
+]])
+
+AT_DATA([bardep.c],
+[[
+int bardep (void);
+int
+bardep (void)
+{
+ return 0;
+}
+]])
+
+AT_DATA([barmod.c],
+[[
+int bardep (void);
+int barsym (void);
+int
+barsym (void)
+{
+ return bardep ();
+}
+]])
+
+: ${LTDLINCL="-I$abs_top_srcdir/libltdl"}
+: ${LIBLTDL="$abs_builddir/../libltdl/libltdlc.la"}
+
+CPPFLAGS="$LTDLINCL $CPPFLAGS"
+inst=`pwd`/inst
+libdir=$inst/lib
+
+AT_CHECK([$CC $CPPFLAGS $CFLAGS -c main.c], [], [ignore], [ignore])
+for file in foomod.c bardep.c barmod.c; do
+ AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c $file],
+ [], [ignore], [ignore])
+done
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o foomod.la ]dnl
+ [-rpath $libdir -module -avoid-version -no-undefined ]dnl
+ [foomod.lo],
+ [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o libbardep.la ]dnl
+ [-rpath $libdir -avoid-version -no-undefined ]dnl
+ [bardep.lo],
+ [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o barmod.la ]dnl
+ [-rpath $libdir -module -avoid-version -no-undefined ]dnl
+ [barmod.lo ./libbardep.la],
+ [], [ignore], [ignore])
+
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o main$EXEEXT ]dnl
+ [main.$OBJEXT $LIBLTDL],
+ [], [ignore], [ignore])
+
+. ./foomod.la
+AT_CHECK([test -n "$dlname" || (exit 77)])
+
+LT_AT_EXEC_CHECK([./main], [], [ignore], [ignore], [./foomod.la no_symbol])
+
+# chmod -x doesn't appear to work in MSYS, and Wine can load no-exec dlls.
+dnl chmod -x $objdir/$dlname
+dnl LT_AT_EXEC_CHECK([./main], [], [ignore], [ignore], [./foomod.la])
+
+rm -f $objdir/$dlname
+LT_AT_EXEC_CHECK([./main], [], [ignore], [ignore], [./foomod.la])
+
+LT_AT_EXEC_CHECK([./main], [], [ignore], [ignore], [./barmod.la no_symbol])
+
+. ./libbardep.la
+# chmod -x doesn't appear to work in MSYS, and Wine can load no-exec dlls.
+dnl chmod -x $objdir/$dlname
+dnl LT_AT_EXEC_CHECK([./main], [], [ignore], [ignore], [./barmod.la])
+
+rm -f $objdir/$dlname
+LT_AT_EXEC_CHECK([./main], [], [ignore], [ignore], [./barmod.la])
+
+AT_CLEANUP
