martin-olivier / dylib

C++ cross-platform wrapper around dynamic loading of shared libraries (dll, so, dylib)
https://conan.io/center/recipes/dylib
MIT License
293 stars 44 forks source link

Errors involving system calls should use the <system_error> mechanisms #32

Closed eyalroz closed 2 years ago

eyalroz commented 2 years ago

C++11 introduced a mechanism - not a fun one, or an easy one to use, but a mechanism - for working with system errors - among others, the ones that, on Unix, you get via errno, and on Windows, with GetLastError() . This is the <system_error> header.

When the error you encounter is one of those - that's what you should throw. See some SO posts about it:

martin-olivier commented 2 years ago

I was thinking to reintroduce an equivalent to dylib::handle_error and dylib::symbol_error to be able to handle correctly each error type, witch is complicated actually since those two exceptions are now std::runtime_error

    /**
     *  This exception is raised when the library failed to load 
     *  or encountered symbol resolution issues
     *
     *  @param message the error message
     */
    class handle_error : public std::runtime_error {
    public:
        explicit handle_error(std::string &&message) : std::runtime_error(std::move(message)) {}
    };

    /**
     *  This exception is raised when the library failed to load a symbol. 
     *  This usually happens when you forgot to put <DYLIB_API> before a library function or variable
     *
     *  @param message the error message
     */
    class symbol_error : public std::runtime_error {
    public:
        explicit symbol_error(std::string &&message) : std::runtime_error(std::move(message)) {}
    };

For example

try {
    dylib lib("foo");
    lib.get_variable<double>("pi_value");
}
catch (const dylib::handle_error &) {
    // handle lib loading failure
}
catch (const dylib::symbol_error &) {
    // handle symbol loading failure
}
eyalroz commented 2 years ago

Well, inheriting std::runtime_error is better than only inheriting std::exception, but I'm always skeptical of introducing new exception classes.

I would try to take inspiration from what other languages use as errors/exceptions when failing to dynamically load compiled code.

martin-olivier commented 2 years ago

I've made the following changes in https://github.com/martin-olivier/dylib/pull/45 :

Unit tests

I've added this test to check the load of a dynamic library from the system library path :

TEST(system_lib, basic_test) {
#if defined(_WIN32) || defined(_WIN64)
    dylib lib("kernel32");
    lib.get_function<DWORD()>("GetCurrentThreadId");
#elif defined(__APPLE__)
    dylib lib("ssh2");
    lib.get_function<const char *(int)>("libssh2_version");
#else
    dylib lib("pthread");
    lib.get_function<int()>("pthread_yield");
#endif
}

Exceptions

load_error
This exception is raised when the library failed to load or the library encountered symbol resolution issues

symbol_error
This exception is raised when the library failed to load a symbol

Those exceptions inherit from std::runtime_error

try {
    dylib lib("foo");
    double pi_value = lib.get_variable<double>("pi_value");
    std::cout << pi_value << std::endl;
}
catch (const dylib::load_error &e) {
    // failed loading "foo" lib
}
catch (const dylib::symbol_error &e) {
    // failed loading "pi_value" symbol
}