From c50784ba0bba2fafff8f65137bc7c84610408cbf Mon Sep 17 00:00:00 2001 From: Lauri Nurmi Date: Sun, 26 Apr 2020 23:43:58 +0300 Subject: [PATCH 1/5] Use the same function for logging dynlib errors on all platforms Reduces the amount of #ifdefs scattered all over the code, thereby simplifying the code. The function was renamed from Error() to ReportError() to emphasize what its purpose is. Error messages logged on *nix are now a bit more verbose, as they are prefixed with our own description text, which were earlier omitted on platforms using the dlxxx() API. --- include/wx/dynlib.h | 8 +++----- src/common/dynlib.cpp | 14 +++----------- src/msw/dlmsw.cpp | 25 +++++++++++++++++++++++++ src/unix/dlunix.cpp | 19 +++++++++++++------ 4 files changed, 44 insertions(+), 22 deletions(-) diff --git a/include/wx/dynlib.h b/include/wx/dynlib.h index 6c791cfaba..3a94ab9d9e 100644 --- a/include/wx/dynlib.h +++ b/include/wx/dynlib.h @@ -357,11 +357,9 @@ protected: // common part of GetSymbol() and HasSymbol() void* DoGetSymbol(const wxString& name, bool* success = NULL) const; -#ifdef HAVE_DLERROR - // log the error after a dlxxx() function failure - static void Error(); -#endif // HAVE_DLERROR - + // log the error after an OS dynamic library function failure + static void ReportError(const wxString& msg, + const wxString& name = wxString()); // the handle to DLL or NULL wxDllType m_handle; diff --git a/src/common/dynlib.cpp b/src/common/dynlib.cpp index 13b0f4544e..f36a6b7efa 100644 --- a/src/common/dynlib.cpp +++ b/src/common/dynlib.cpp @@ -86,11 +86,7 @@ bool wxDynamicLibrary::Load(const wxString& libnameOrig, int flags) if ( m_handle == 0 && !(flags & wxDL_QUIET) ) { -#ifdef HAVE_DLERROR - Error(); -#else - wxLogSysError(_("Failed to load shared library '%s'"), libname.c_str()); -#endif + ReportError(_("Failed to load shared library '%s'"), libname); } return IsLoaded(); @@ -114,12 +110,8 @@ void *wxDynamicLibrary::GetSymbol(const wxString& name, bool *success) const void *symbol = DoGetSymbol(name, success); if ( !symbol ) { -#ifdef HAVE_DLERROR - Error(); -#else - wxLogSysError(_("Couldn't find symbol '%s' in a dynamic library"), - name.c_str()); -#endif + ReportError(_("Couldn't find symbol '%s' in a dynamic library"), + name.c_str()); } return symbol; diff --git a/src/msw/dlmsw.cpp b/src/msw/dlmsw.cpp index d664c6ae4b..d5916d408f 100644 --- a/src/msw/dlmsw.cpp +++ b/src/msw/dlmsw.cpp @@ -145,6 +145,31 @@ wxDllType wxDynamicLibrary::GetProgramHandle() return (wxDllType)::GetModuleHandle(NULL); } +// ---------------------------------------------------------------------------- +// error handling +// ---------------------------------------------------------------------------- + +/* static */ +void wxDynamicLibrary::ReportError(const wxString& message, const wxString& name) +{ + wxString msg(message); + if ( name.IsEmpty() && msg.Find("%s") == wxNOT_FOUND ) + msg += "%s"; + // msg needs a %s for the name + wxASSERT(msg.Find("%s") != wxNOT_FOUND); + + const unsigned long code = wxSysErrorCode(); + wxString errMsg = wxSysErrorMsgStr(code); + + // The error message (specifically code==193) may contain a + // placeholder '%1' which stands for the filename. + errMsg.Replace("%1", name, false); + + // Mimic the output of wxLogSysError(), but use our pre-processed + // errMsg. + wxLogError(msg + " " + _("(error %d: %s)"), name, code, errMsg); +} + // ---------------------------------------------------------------------------- // loading/unloading DLLs // ---------------------------------------------------------------------------- diff --git a/src/unix/dlunix.cpp b/src/unix/dlunix.cpp index 325d029c02..66606df70d 100644 --- a/src/unix/dlunix.cpp +++ b/src/unix/dlunix.cpp @@ -120,8 +120,8 @@ void wxDynamicLibrary::Unload(wxDllType handle) #if defined(USE_POSIX_DL_FUNCS) && defined(HAVE_DLERROR) if ( rc != 0 ) - Error(); #endif + ReportError(_("Failed to unload shared library")); } /* static */ @@ -145,20 +145,27 @@ void *wxDynamicLibrary::RawGetSymbol(wxDllType handle, const wxString& name) // error handling // ---------------------------------------------------------------------------- -#ifdef HAVE_DLERROR - /* static */ -void wxDynamicLibrary::Error() +void wxDynamicLibrary::ReportError(const wxString& message, + const wxString& name) { + wxString msg(message); + if ( name.IsEmpty() && msg.Find("%s") == wxNOT_FOUND ) + msg += "%s"; + // msg needs a %s for the name + wxASSERT(msg.Find("%s") != wxNOT_FOUND); +#ifdef HAVE_DLERROR wxString err(dlerror()); if ( err.empty() ) err = _("Unknown dynamic library error"); - wxLogError(wxT("%s"), err); + wxLogError(msg + wxT(": %s"), name, err); +#else // !HAVE_DLERROR + wxLogSysError(msg, name); +#endif // HAVE_DLERROR } -#endif // HAVE_DLERROR // ---------------------------------------------------------------------------- // listing loaded modules From e289eb07e1cb3091670e154ac10bc9eebda0d783 Mon Sep 17 00:00:00 2001 From: Lauri Nurmi Date: Wed, 13 May 2020 09:30:40 +0300 Subject: [PATCH 2/5] Get rid of non-POSIX code for loading dynlibs on *nix The alternative, (non-POSIX) shl_xxx() API is/was apparently available on HP-UX, but even there the POSIX dlxxx() functions have been the preferred way to load libraries since the past ~20 years. --- src/unix/dlunix.cpp | 51 ++++----------------------------------------- 1 file changed, 4 insertions(+), 47 deletions(-) diff --git a/src/unix/dlunix.cpp b/src/unix/dlunix.cpp index 66606df70d..8686449d34 100644 --- a/src/unix/dlunix.cpp +++ b/src/unix/dlunix.cpp @@ -50,9 +50,7 @@ #endif -#if defined(HAVE_DLOPEN) - #define USE_POSIX_DL_FUNCS -#elif !defined(HAVE_SHL_LOAD) +#ifndef HAVE_DLOPEN #error "Don't know how to load dynamic libraries on this platform!" #endif @@ -66,11 +64,7 @@ wxDllType wxDynamicLibrary::GetProgramHandle() { -#ifdef USE_POSIX_DL_FUNCS return dlopen(0, RTLD_LAZY); -#else - return PROG_HANDLE; -#endif } /* static */ @@ -79,7 +73,6 @@ wxDllType wxDynamicLibrary::RawLoad(const wxString& libname, int flags) wxASSERT_MSG( !(flags & wxDL_NOW) || !(flags & wxDL_LAZY), wxT("wxDL_LAZY and wxDL_NOW are mutually exclusive.") ); -#ifdef USE_POSIX_DL_FUNCS // we need to use either RTLD_NOW or RTLD_LAZY because if we call dlopen() // with flags == 0 recent versions of glibc just fail the call, so use // RTLD_NOW even if wxDL_NOW was not specified @@ -89,54 +82,21 @@ wxDllType wxDynamicLibrary::RawLoad(const wxString& libname, int flags) rtldFlags |= RTLD_GLOBAL; return dlopen(libname.fn_str(), rtldFlags); -#else // !USE_POSIX_DL_FUNCS - int shlFlags = 0; - - if ( flags & wxDL_LAZY ) - { - shlFlags |= BIND_DEFERRED; - } - else if ( flags & wxDL_NOW ) - { - shlFlags |= BIND_IMMEDIATE; - } - - return shl_load(libname.fn_str(), shlFlags, 0); -#endif // USE_POSIX_DL_FUNCS/!USE_POSIX_DL_FUNCS } /* static */ void wxDynamicLibrary::Unload(wxDllType handle) { -#ifdef HAVE_DLERROR - int rc = -#endif + int rc = dlclose(handle); -#ifdef USE_POSIX_DL_FUNCS - dlclose(handle); -#else // !USE_POSIX_DL_FUNCS - shl_unload(handle); -#endif // USE_POSIX_DL_FUNCS/!USE_POSIX_DL_FUNCS - -#if defined(USE_POSIX_DL_FUNCS) && defined(HAVE_DLERROR) if ( rc != 0 ) -#endif ReportError(_("Failed to unload shared library")); } /* static */ void *wxDynamicLibrary::RawGetSymbol(wxDllType handle, const wxString& name) { - void *symbol; - -#ifdef USE_POSIX_DL_FUNCS - symbol = dlsym(handle, name.fn_str()); -#else // !USE_POSIX_DL_FUNCS - // note that shl_findsym modifies the handle argument to indicate where the - // symbol was found, but it's ok to modify the local handle copy here - if ( shl_findsym(&handle, name.fn_str(), TYPE_UNDEFINED, &symbol) != 0 ) - symbol = 0; -#endif // USE_POSIX_DL_FUNCS/!USE_POSIX_DL_FUNCS + void *symbol = dlsym(handle, name.fn_str()); return symbol; } @@ -154,16 +114,13 @@ void wxDynamicLibrary::ReportError(const wxString& message, msg += "%s"; // msg needs a %s for the name wxASSERT(msg.Find("%s") != wxNOT_FOUND); -#ifdef HAVE_DLERROR + wxString err(dlerror()); if ( err.empty() ) err = _("Unknown dynamic library error"); wxLogError(msg + wxT(": %s"), name, err); -#else // !HAVE_DLERROR - wxLogSysError(msg, name); -#endif // HAVE_DLERROR } From 2e6fec3601f5fde8453d011594a155935297fa27 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 14 May 2020 23:53:45 +0200 Subject: [PATCH 3/5] Remove unnecessary c_str() call No real changes. --- src/common/dynlib.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/common/dynlib.cpp b/src/common/dynlib.cpp index f36a6b7efa..37fea09768 100644 --- a/src/common/dynlib.cpp +++ b/src/common/dynlib.cpp @@ -110,8 +110,7 @@ void *wxDynamicLibrary::GetSymbol(const wxString& name, bool *success) const void *symbol = DoGetSymbol(name, success); if ( !symbol ) { - ReportError(_("Couldn't find symbol '%s' in a dynamic library"), - name.c_str()); + ReportError(_("Couldn't find symbol '%s' in a dynamic library"), name); } return symbol; From fea8c608b1368b7a0839a4642a599d2768a21c8a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 14 May 2020 23:56:03 +0200 Subject: [PATCH 4/5] Remove checks for shl_load() from configure/CMake This function is not used any more since e289eb07e1 (Get rid of non-POSIX code for loading dynlibs on *nix, 2020-05-13), so don't check for it and don't defined the corresponding HAVE_SHL_LOAD symbol. --- build/cmake/setup.cmake | 2 -- build/cmake/setup.h.in | 3 -- configure | 64 ----------------------------------------- configure.in | 14 --------- include/wx/dynlib.h | 3 -- setup.h.in | 3 -- setup.h_vms | 3 -- 7 files changed, 92 deletions(-) diff --git a/build/cmake/setup.cmake b/build/cmake/setup.cmake index deb456924b..c3bfa21f55 100644 --- a/build/cmake/setup.cmake +++ b/build/cmake/setup.cmake @@ -616,8 +616,6 @@ cmake_pop_check_state() if(HAVE_DLOPEN) check_symbol_exists(dlerror dlfcn.h HAVE_DLERROR) check_symbol_exists(dladdr dlfcn.h HAVE_DLADDR) -else() - check_symbol_exists(shl_load dl.h HAVE_SHL_LOAD) endif() check_function_exists(gettimeofday HAVE_GETTIMEOFDAY) diff --git a/build/cmake/setup.h.in b/build/cmake/setup.h.in index c18154227b..0c9b66537d 100644 --- a/build/cmake/setup.h.in +++ b/build/cmake/setup.h.in @@ -984,9 +984,6 @@ /* Define if you have pthread_attr_setstacksize */ #cmakedefine HAVE_PTHREAD_ATTR_SETSTACKSIZE 1 -/* Define if you have shl_load() */ -#cmakedefine HAVE_SHL_LOAD 1 - /* Define if you have snprintf() */ #cmakedefine HAVE_SNPRINTF 1 diff --git a/configure b/configure index fd5c48a680..9e469df421 100755 --- a/configure +++ b/configure @@ -32736,70 +32736,6 @@ if test "x$ac_cv_lib_dl_dlopen" = xyes; then : HAVE_DL_FUNCS=1 DL_LINK="-ldl $DL_LINK" -else - - for ac_func in shl_load -do : - ac_fn_c_check_func "$LINENO" "shl_load" "ac_cv_func_shl_load" -if test "x$ac_cv_func_shl_load" = xyes; then : - cat >>confdefs.h <<_ACEOF -#define HAVE_SHL_LOAD 1 -_ACEOF - - $as_echo "#define HAVE_SHL_LOAD 1" >>confdefs.h - - HAVE_SHL_FUNCS=1 - -else - - { $as_echo "$as_me:${as_lineno-$LINENO}: checking for dld in -lshl_load" >&5 -$as_echo_n "checking for dld in -lshl_load... " >&6; } -if ${ac_cv_lib_shl_load_dld+:} false; then : - $as_echo_n "(cached) " >&6 -else - ac_check_lib_save_LIBS=$LIBS -LIBS="-lshl_load $LIBS" -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -/* Override any GCC internal prototype to avoid an error. - Use char because int might match the return type of a GCC - builtin and then its argument prototype would still apply. */ -#ifdef __cplusplus -extern "C" -#endif -char dld (); -int -main () -{ -return dld (); - ; - return 0; -} -_ACEOF -if ac_fn_c_try_link "$LINENO"; then : - ac_cv_lib_shl_load_dld=yes -else - ac_cv_lib_shl_load_dld=no -fi -rm -f core conftest.err conftest.$ac_objext \ - conftest$ac_exeext conftest.$ac_ext -LIBS=$ac_check_lib_save_LIBS -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_shl_load_dld" >&5 -$as_echo "$ac_cv_lib_shl_load_dld" >&6; } -if test "x$ac_cv_lib_shl_load_dld" = xyes; then : - - HAVE_SHL_FUNCS=1 - DL_LINK="-ldld $DL_LINK" - -fi - - -fi -done - - fi diff --git a/configure.in b/configure.in index c45f68fff5..6c521274c6 100644 --- a/configure.in +++ b/configure.in @@ -5385,20 +5385,6 @@ if test "$TOOLKIT" != "MSW"; then AC_DEFINE(HAVE_DLOPEN) HAVE_DL_FUNCS=1 DL_LINK="-ldl $DL_LINK" - ], - [ - AC_CHECK_FUNCS(shl_load, - [ - AC_DEFINE(HAVE_SHL_LOAD) - HAVE_SHL_FUNCS=1 - ], - [ - AC_CHECK_LIB(shl_load, dld, - [ - HAVE_SHL_FUNCS=1 - DL_LINK="-ldld $DL_LINK" - ]) - ]) ]) ]) diff --git a/include/wx/dynlib.h b/include/wx/dynlib.h index 3a94ab9d9e..1036771c06 100644 --- a/include/wx/dynlib.h +++ b/include/wx/dynlib.h @@ -29,9 +29,6 @@ class WXDLLIMPEXP_FWD_BASE wxDynamicLibraryDetailsCreator; #elif defined(HAVE_DLOPEN) #include typedef void *wxDllType; -#elif defined(HAVE_SHL_LOAD) - #include - typedef shl_t wxDllType; #elif defined(__WXMAC__) #include typedef CFragConnectionID wxDllType; diff --git a/setup.h.in b/setup.h.in index 26ed7a7d14..b192d1d84b 100644 --- a/setup.h.in +++ b/setup.h.in @@ -984,9 +984,6 @@ /* Define if you have pthread_attr_setstacksize */ #undef HAVE_PTHREAD_ATTR_SETSTACKSIZE -/* Define if you have shl_load() */ -#undef HAVE_SHL_LOAD - /* Define if you have snprintf() */ #undef HAVE_SNPRINTF diff --git a/setup.h_vms b/setup.h_vms index 92efe12b49..42461b9b04 100644 --- a/setup.h_vms +++ b/setup.h_vms @@ -1076,9 +1076,6 @@ typedef pid_t GPid; #undef HAVE_SETENV #endif -/* Define if you have shl_load() */ -#undef HAVE_SHL_LOAD - #if __CRTL_VER >= 70312000 /* Define if you have snprintf() */ #define HAVE_SNPRINTF 1 From 7a82a0bbf5f70fc1a4a6617c4c05dda82630a200 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 15 May 2020 00:00:06 +0200 Subject: [PATCH 5/5] Assume dlerror() function is always available when dlopen() is Remove separate checks for dlerror() which don't seem to be needed under any platform any longer. No real changes, just slim down configure/CMake a tiny bit. --- build/cmake/setup.cmake | 1 - build/cmake/setup.h.in | 3 -- configure | 57 ----------------------------------- configure.in | 6 ---- include/wx/osx/config_xcode.h | 1 - setup.h.in | 3 -- setup.h_vms | 3 -- 7 files changed, 74 deletions(-) diff --git a/build/cmake/setup.cmake b/build/cmake/setup.cmake index c3bfa21f55..7808705b74 100644 --- a/build/cmake/setup.cmake +++ b/build/cmake/setup.cmake @@ -614,7 +614,6 @@ set(CMAKE_REQUIRED_LIBRARIES dl) check_symbol_exists(dlopen dlfcn.h HAVE_DLOPEN) cmake_pop_check_state() if(HAVE_DLOPEN) - check_symbol_exists(dlerror dlfcn.h HAVE_DLERROR) check_symbol_exists(dladdr dlfcn.h HAVE_DLADDR) endif() check_function_exists(gettimeofday HAVE_GETTIMEOFDAY) diff --git a/build/cmake/setup.h.in b/build/cmake/setup.h.in index 0c9b66537d..613cef5ba5 100644 --- a/build/cmake/setup.h.in +++ b/build/cmake/setup.h.in @@ -1086,9 +1086,6 @@ /* Define if wchar_t is distinct type in your compiler. */ #cmakedefine wxWCHAR_T_IS_REAL_TYPE 1 -/* Define if you have the dlerror function. */ -#cmakedefine HAVE_DLERROR 1 - /* Define if you have the dladdr function. */ #cmakedefine HAVE_DLADDR 1 diff --git a/configure b/configure index 9e469df421..9b6e686b28 100755 --- a/configure +++ b/configure @@ -32744,63 +32744,6 @@ done if test "$HAVE_DL_FUNCS" = 1; then - for ac_func in dlerror -do : - ac_fn_c_check_func "$LINENO" "dlerror" "ac_cv_func_dlerror" -if test "x$ac_cv_func_dlerror" = xyes; then : - cat >>confdefs.h <<_ACEOF -#define HAVE_DLERROR 1 -_ACEOF - $as_echo "#define HAVE_DLERROR 1" >>confdefs.h - -else - - { $as_echo "$as_me:${as_lineno-$LINENO}: checking for dlerror in -ldl" >&5 -$as_echo_n "checking for dlerror in -ldl... " >&6; } -if ${ac_cv_lib_dl_dlerror+:} false; then : - $as_echo_n "(cached) " >&6 -else - ac_check_lib_save_LIBS=$LIBS -LIBS="-ldl $LIBS" -cat confdefs.h - <<_ACEOF >conftest.$ac_ext -/* end confdefs.h. */ - -/* Override any GCC internal prototype to avoid an error. - Use char because int might match the return type of a GCC - builtin and then its argument prototype would still apply. */ -#ifdef __cplusplus -extern "C" -#endif -char dlerror (); -int -main () -{ -return dlerror (); - ; - return 0; -} -_ACEOF -if ac_fn_c_try_link "$LINENO"; then : - ac_cv_lib_dl_dlerror=yes -else - ac_cv_lib_dl_dlerror=no -fi -rm -f core conftest.err conftest.$ac_objext \ - conftest$ac_exeext conftest.$ac_ext -LIBS=$ac_check_lib_save_LIBS -fi -{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_dl_dlerror" >&5 -$as_echo "$ac_cv_lib_dl_dlerror" >&6; } -if test "x$ac_cv_lib_dl_dlerror" = xyes; then : - $as_echo "#define HAVE_DLERROR 1" >>confdefs.h - -fi - - - -fi -done - for ac_func in dladdr do : ac_fn_c_check_func "$LINENO" "dladdr" "ac_cv_func_dladdr" diff --git a/configure.in b/configure.in index 6c521274c6..4bf3edd290 100644 --- a/configure.in +++ b/configure.in @@ -5390,12 +5390,6 @@ if test "$TOOLKIT" != "MSW"; then dnl check also for dlerror() if test "$HAVE_DL_FUNCS" = 1; then - AC_CHECK_FUNCS(dlerror, - AC_DEFINE(HAVE_DLERROR), - [ - AC_CHECK_LIB(dl, dlerror, AC_DEFINE(HAVE_DLERROR)) - ] - ) AC_CHECK_FUNCS(dladdr, AC_DEFINE(HAVE_DLADDR), [ diff --git a/include/wx/osx/config_xcode.h b/include/wx/osx/config_xcode.h index a1ceef066a..cafe26808e 100644 --- a/include/wx/osx/config_xcode.h +++ b/include/wx/osx/config_xcode.h @@ -87,7 +87,6 @@ #define SIZEOF_LONG_LONG 8 #define wxSIZE_T_IS_ULONG 1 #define wxWCHAR_T_IS_REAL_TYPE 1 -#define HAVE_DLERROR 1 #define HAVE_FCNTL 1 #define HAVE_GETHOSTBYNAME 1 #define HAVE_GETSERVBYNAME 1 diff --git a/setup.h.in b/setup.h.in index b192d1d84b..59a8fce331 100644 --- a/setup.h.in +++ b/setup.h.in @@ -1086,9 +1086,6 @@ /* Define if wchar_t is distinct type in your compiler. */ #undef wxWCHAR_T_IS_REAL_TYPE -/* Define if you have the dlerror function. */ -#undef HAVE_DLERROR - /* Define if you have the dladdr function. */ #undef HAVE_DLADDR diff --git a/setup.h_vms b/setup.h_vms index 42461b9b04..ddb49c8cce 100644 --- a/setup.h_vms +++ b/setup.h_vms @@ -1194,9 +1194,6 @@ typedef pid_t GPid; /* Define if wchar_t is distinct type in your compiler. */ #define wxWCHAR_T_IS_REAL_TYPE 1 -/* Define if you have the dlerror function. */ -#define HAVE_DLERROR 1 - /* Define if you have the dladdr function. */ #undef HAVE_DLADDR