diff --git a/.clang-tidy b/.clang-tidy index ae6716f..af7bba0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -16,6 +16,7 @@ Checks: > bugprone-signed-char-misuse, bugprone-sizeof-expression, bugprone-branch-clone, + misc-include-cleaner, -clang-analyzer-security.insecureAPI.*, -misc-no-recursion, diff --git a/examples/call_ivp_from_python.py b/examples/call_ivp_from_python.py index f183abf..dcf5881 100755 --- a/examples/call_ivp_from_python.py +++ b/examples/call_ivp_from_python.py @@ -29,7 +29,7 @@ def rhs(t, y, ydot): def main(): args = _parse_args() impl = args.impl - print("Calling from Python an open interface for quadratic solver") + print("Calling from Python an open interface for initial-value problems") print(f"Implementation: {impl}") s = IVP(impl) t0 = 0.0 diff --git a/oif/dispatch.c b/oif/dispatch.c index 0efeca0..90c5f83 100644 --- a/oif/dispatch.c +++ b/oif/dispatch.c @@ -1,18 +1,22 @@ // Dispatch library that is called from other languages, and dispatches it // to the appropriate language-specific dispatch. -#include "oif/api.h" -#include #include -#include #include +#include #include #include #include +// This is required to avoid clang-tidy issues with `hashmap.h`. +#if __STDC_VERSION__ < 202300L +#define typeof __typeof__ +#endif + #include -#include -#include +#include "oif/api.h" +#include "oif/dispatch.h" +#include "oif/dispatch_api.h" char OIF_DISPATCH_C_SO[] = "liboif_dispatch_c.so"; char OIF_DISPATCH_PYTHON_SO[] = "liboif_dispatch_python.so"; @@ -25,20 +29,21 @@ static const char *OIF_IMPL_ROOT_DIR; */ void *OIF_DISPATCH_HANDLES[OIF_LANG_COUNT]; +// cppcheck-suppress unusedStructMember static HASHMAP(ImplHandle, ImplInfo) IMPL_MAP; -static bool _INITIALIZED = false; +static bool INITIALIZED_ = false; -static int _IMPL_COUNTER = 1000; +static int IMPL_COUNTER_ = 1000; size_t hash_fn(const ImplHandle *key) { - size_t result = *key; - if (result < 0) { - result = -result; + if (*key < 0) { + fprintf(stderr, "[dispatch] Was expecting a non-negative number\n"); + exit(1); } - return result % SIZE_MAX; + return *key; } int @@ -58,7 +63,7 @@ init_module_(void) return -1; } hashmap_init(&IMPL_MAP, hash_fn, compare_fn); - _INITIALIZED = true; + INITIALIZED_ = true; return 0; } @@ -67,7 +72,7 @@ ImplHandle load_interface_impl(const char *interface, const char *impl, size_t version_major, size_t version_minor) { - if (!_INITIALIZED) { + if (!INITIALIZED_) { int status = init_module_(); if (status) { return -1; @@ -75,6 +80,11 @@ load_interface_impl(const char *interface, const char *impl, size_t version_majo } DispatchHandle dh; const char *dispatch_lang_so; + void *lib_handle = NULL; + FILE *conf_file; + char *buffer; + /* One must be a pessimist, while programming in C. */ + ImplHandle retval = OIF_IMPL_INIT_ERROR; char conf_filename[1024] = ""; strcat(conf_filename, OIF_IMPL_ROOT_DIR); @@ -86,7 +96,7 @@ load_interface_impl(const char *interface, const char *impl, size_t version_majo strcat(conf_filename, impl); strcat(conf_filename, ".conf"); - FILE *conf_file = fopen(conf_filename, "r"); + conf_file = fopen(conf_filename, "re"); if (conf_file == NULL) { fprintf(stderr, "[dispatch] Cannot load conf file '%s'\n", conf_filename); return -1; @@ -96,28 +106,29 @@ load_interface_impl(const char *interface, const char *impl, size_t version_majo } // Temporary buffer to read lines from file. - const size_t buffer_size = 512; - int len; - char *buffer = malloc(buffer_size * sizeof(char)); + const int buffer_size = 512; + size_t len; + char *fgets_status; + buffer = malloc(sizeof(char) * buffer_size); if (buffer == NULL) { fprintf(stderr, "[dispatch] Could not allocate buffer for parsing " "implementation configuration files\n"); - exit(1); + goto cleanup; } char backend_name[16]; - buffer = fgets(buffer, buffer_size, conf_file); - if (buffer == NULL) { + fgets_status = fgets(buffer, buffer_size, conf_file); + if (fgets_status == NULL) { fprintf(stderr, "[dispatch] Could not read backend line from configuration " "file '%s'\n", conf_filename); - return -1; + goto cleanup; } len = strlen(buffer); if (buffer[len - 1] != '\n') { fprintf(stderr, "Backend name is longer than allocated buffer\n"); - return -1; + goto cleanup; } else { // Trim the new line character. @@ -126,17 +137,17 @@ load_interface_impl(const char *interface, const char *impl, size_t version_majo strcpy(backend_name, buffer); fprintf(stderr, "[dispatch] Backend name: %s\n", backend_name); - buffer = fgets(buffer, buffer_size, conf_file); - if (buffer == NULL) { + fgets_status = fgets(buffer, buffer_size, conf_file); + if (fgets_status == NULL) { fprintf(stderr, "[dispatch] Could not read implementation details line " "from the configuration file\n"); - return -1; + goto cleanup; } len = strlen(buffer); if (buffer[len - 1] != '\n') { - fprintf(stderr, "Backend name is longer than allocated array\n"); - exit(EXIT_FAILURE); + fprintf(stderr, "[dispatch] Backend name is longer than allocated array\n"); + goto cleanup; } else { // Trim new line character. @@ -145,7 +156,6 @@ load_interface_impl(const char *interface, const char *impl, size_t version_majo char impl_details[512]; strcpy(impl_details, buffer); fprintf(stderr, "[dispatch] Implementation details: '%s'\n", impl_details); - free(buffer); if (strcmp(backend_name, "c") == 0) { dh = OIF_LANG_C; @@ -157,16 +167,15 @@ load_interface_impl(const char *interface, const char *impl, size_t version_majo } else { fprintf(stderr, "[dispatch] Implementation has unknown backend: '%s'\n", backend_name); - exit(EXIT_FAILURE); + goto cleanup; } - void *lib_handle; if (OIF_DISPATCH_HANDLES[dh] == NULL) { lib_handle = dlopen(dispatch_lang_so, RTLD_LOCAL | RTLD_LAZY); if (lib_handle == NULL) { fprintf(stderr, "[dispatch] Cannot load shared library '%s'\n", dispatch_lang_so); fprintf(stderr, "Error message: %s\n", dlerror()); - exit(EXIT_FAILURE); + goto cleanup; } OIF_DISPATCH_HANDLES[dh] = lib_handle; } @@ -174,24 +183,34 @@ load_interface_impl(const char *interface, const char *impl, size_t version_majo lib_handle = OIF_DISPATCH_HANDLES[dh]; } - ImplInfo *(*load_backend_fn)(const char *, size_t, size_t); - load_backend_fn = dlsym(lib_handle, "load_backend"); + ImplInfo *(*load_impl_fn)(const char *, size_t, size_t); + load_impl_fn = dlsym(lib_handle, "load_impl"); - if (load_backend_fn == NULL) { - fprintf(stderr, "[dispatch] Could not load function %s: %s\n", "load_backend", - dlerror()); + if (load_impl_fn == NULL) { + fprintf(stderr, "[dispatch] Could not load function %s: %s\n", "load_impl", dlerror()); + goto cleanup; } - ImplInfo *impl_info = load_backend_fn(impl_details, version_major, version_minor); + ImplInfo *impl_info = load_impl_fn(impl_details, version_major, version_minor); if (impl_info == NULL) { fprintf(stderr, "[dispatch] Could not load implementation\n"); - return OIF_IMPL_INIT_ERROR; + goto cleanup; } - impl_info->implh = _IMPL_COUNTER; - _IMPL_COUNTER++; + impl_info->implh = IMPL_COUNTER_; + IMPL_COUNTER_++; impl_info->dh = dh; hashmap_put(&IMPL_MAP, &impl_info->implh, impl_info); - return impl_info->implh; + retval = impl_info->implh; + +cleanup: + if (buffer != NULL) { + free(buffer); + } + if (conf_file != NULL) { + fclose(conf_file); + } + + return retval; } int @@ -224,7 +243,7 @@ unload_interface_impl(ImplHandle implh) } int -call_interface_method(ImplHandle implh, const char *method, OIFArgs *args, OIFArgs *retvals) +call_interface_impl(ImplHandle implh, const char *method, OIFArgs *in_args, OIFArgs *out_args) { int status; @@ -239,16 +258,16 @@ call_interface_method(ImplHandle implh, const char *method, OIFArgs *args, OIFAr } void *lib_handle = OIF_DISPATCH_HANDLES[dh]; - int (*run_interface_method_fn)(ImplInfo *, const char *, OIFArgs *, OIFArgs *); - run_interface_method_fn = dlsym(lib_handle, "run_interface_method"); - if (run_interface_method_fn == NULL) { + int (*call_impl_fn)(ImplInfo *, const char *, OIFArgs *, OIFArgs *); + call_impl_fn = dlsym(lib_handle, "call_impl"); + if (call_impl_fn == NULL) { fprintf(stderr, - "[dispatch] Could not load function 'run_interface_method' " + "[dispatch] Could not load function 'call_impl' " "for language id '%u'\n", dh); return -1; } - status = run_interface_method_fn(impl_info, method, args, retvals); + status = call_impl_fn(impl_info, method, in_args, out_args); if (status) { fprintf(stderr, diff --git a/oif/include/oif/api.h b/oif/include/oif/api.h index bd3a9f4..02a3103 100644 --- a/oif/include/oif/api.h +++ b/oif/include/oif/api.h @@ -39,6 +39,6 @@ typedef struct { } OIFCallback; enum { - OIF_ERROR = 101, - OIF_IMPL_INIT_ERROR = 102, + OIF_ERROR = -1, + OIF_IMPL_INIT_ERROR = -2, }; diff --git a/oif/include/oif/dispatch.h b/oif/include/oif/dispatch.h index 71217b8..c9c0e49 100644 --- a/oif/include/oif/dispatch.h +++ b/oif/include/oif/dispatch.h @@ -30,6 +30,14 @@ load_interface_impl(const char *interface, const char *impl, size_t version_majo int unload_interface_impl(ImplHandle implh); +/** + * Call implementation of an interface. + * @param implh Implementat handle that identifies the implementation + * @param method Name of the method (function) to invoke + * @param in_args Array of input arguments + * @param out_args Array of output arguments + * @return status code that signals about an error if non-zero + */ int -call_interface_method(ImplHandle implh, const char *method, OIFArgs *args, OIFArgs *retvals); +call_interface_impl(ImplHandle implh, const char *method, OIFArgs *in_args, OIFArgs *out_args); #endif diff --git a/oif/include/oif/dispatch_api.h b/oif/include/oif/dispatch_api.h index d4223ca..289f48b 100644 --- a/oif/include/oif/dispatch_api.h +++ b/oif/include/oif/dispatch_api.h @@ -28,11 +28,10 @@ typedef struct { } ImplInfo; ImplInfo * -load_backend(const char *impl_details, size_t version_major, size_t version_minor); +load_impl(const char *impl_details, size_t version_major, size_t version_minor); int unload_impl(ImplInfo *impl_info); int -run_interface_method(ImplInfo *impl_info, const char *method, OIFArgs *in_args, - OIFArgs *out_args); +call_impl(ImplInfo *impl_info, const char *method, OIFArgs *in_args, OIFArgs *out_args); diff --git a/oif/interfaces/c/src/ivp.c b/oif/interfaces/c/src/ivp.c index ae407f5..a2ca209 100644 --- a/oif/interfaces/c/src/ivp.c +++ b/oif/interfaces/c/src/ivp.c @@ -26,7 +26,7 @@ oif_ivp_set_rhs_fn(ImplHandle implh, oif_ivp_rhs_fn_t rhs) .arg_values = out_arg_values, }; - int status = call_interface_method(implh, "set_rhs_fn", &in_args, &out_args); + int status = call_interface_impl(implh, "set_rhs_fn", &in_args, &out_args); return status; } @@ -50,7 +50,7 @@ oif_ivp_set_initial_value(ImplHandle implh, OIFArrayF64 *y0, double t0) .arg_values = out_arg_values, }; - int status = call_interface_method(implh, "set_initial_value", &in_args, &out_args); + int status = call_interface_impl(implh, "set_initial_value", &in_args, &out_args); return status; } @@ -74,7 +74,7 @@ oif_ivp_integrate(ImplHandle implh, double t, OIFArrayF64 *y) .arg_values = out_arg_values, }; - int status = call_interface_method(implh, "integrate", &in_args, &out_args); + int status = call_interface_impl(implh, "integrate", &in_args, &out_args); return status; } diff --git a/oif/interfaces/c/src/linsolve.c b/oif/interfaces/c/src/linsolve.c index 24f6fc5..3bb6324 100644 --- a/oif/interfaces/c/src/linsolve.c +++ b/oif/interfaces/c/src/linsolve.c @@ -23,7 +23,7 @@ oif_solve_linear_system(ImplHandle implh, OIFArrayF64 *A, OIFArrayF64 *b, OIFArr .arg_values = out_arg_values, }; - int status = call_interface_method(implh, "solve_lin", &in_args, &out_args); + int status = call_interface_impl(implh, "solve_lin", &in_args, &out_args); return status; } diff --git a/oif/interfaces/c/src/qeq.c b/oif/interfaces/c/src/qeq.c index 2d8cebf..48e28ee 100644 --- a/oif/interfaces/c/src/qeq.c +++ b/oif/interfaces/c/src/qeq.c @@ -23,7 +23,7 @@ oif_solve_qeq(ImplHandle implh, double a, double b, double c, OIFArrayF64 *roots .arg_values = out_arg_values, }; - int status = call_interface_method(implh, "solve_qeq", &in_args, &out_args); + int status = call_interface_impl(implh, "solve_qeq", &in_args, &out_args); return status; } diff --git a/oif/interfaces/python/oif/interfaces/ivp.py b/oif/interfaces/python/oif/interfaces/ivp.py index bb4cbf3..a296826 100644 --- a/oif/interfaces/python/oif/interfaces/ivp.py +++ b/oif/interfaces/python/oif/interfaces/ivp.py @@ -45,4 +45,5 @@ def print_stats(self): self._binding.call("print_stats", (), ()) def __del__(self): - unload_impl(self._binding) + if hasattr(self, "_binding"): + unload_impl(self._binding) diff --git a/oif/interfaces/python/oif/interfaces/linear_solver.py b/oif/interfaces/python/oif/interfaces/linear_solver.py index 4e2bd72..7ee86d9 100644 --- a/oif/interfaces/python/oif/interfaces/linear_solver.py +++ b/oif/interfaces/python/oif/interfaces/linear_solver.py @@ -12,4 +12,5 @@ def solve(self, A: np.ndarray, b: np.ndarray) -> np.ndarray: return result def __del__(self): - unload_impl(self._binding) + if hasattr(self, "_binding"): + unload_impl(self._binding) diff --git a/oif/interfaces/python/oif/interfaces/qeq_solver.py b/oif/interfaces/python/oif/interfaces/qeq_solver.py index bb377de..c21693d 100644 --- a/oif/interfaces/python/oif/interfaces/qeq_solver.py +++ b/oif/interfaces/python/oif/interfaces/qeq_solver.py @@ -12,4 +12,5 @@ def solve(self, a: float, b: float, c: float): return result def __del__(self): - unload_impl(self._binding) + if hasattr(self, "_binding"): + unload_impl(self._binding) diff --git a/oif/lang_python/oif/core.py b/oif/lang_python/oif/core.py index 1a844b4..4fb0c78 100644 --- a/oif/lang_python/oif/core.py +++ b/oif/lang_python/oif/core.py @@ -223,9 +223,9 @@ def call(self, method, user_args, out_user_args): ) out_packed = OIFArgs(num_out_args, out_arg_types_ctypes, out_arg_values_ctypes) - call_interface_method = wrap_c_function( + call_interface_impl = _wrap_c_function( _lib_dispatch, - "call_interface_method", + "call_interface_impl", ctypes.c_int, [ ctypes.c_int, @@ -234,7 +234,7 @@ def call(self, method, user_args, out_user_args): ctypes.POINTER(OIFArgs), ], ) - status = call_interface_method( + status = call_interface_impl( self.implh, method.encode(), ctypes.byref(in_args_packed), @@ -248,7 +248,7 @@ def call(self, method, user_args, out_user_args): def init_impl(interface: str, impl: str, major: UInt, minor: UInt): - load_interface_impl = wrap_c_function( + load_interface_impl = _wrap_c_function( _lib_dispatch, "load_interface_impl", ctypes.c_int, @@ -256,12 +256,16 @@ def init_impl(interface: str, impl: str, major: UInt, minor: UInt): ) implh = load_interface_impl(interface.encode(), impl.encode(), major, minor) if implh < 0: - raise RuntimeError("Cannot initialize backend") + raise RuntimeError( + "Error occurred " + f"during initialization of the implementation '{impl}' " + f"for the interface '{interface}'" + ) return OIFPyBinding(implh, interface, impl) def unload_impl(binding: OIFPyBinding): - unload_interface_impl = wrap_c_function( + unload_interface_impl = _wrap_c_function( _lib_dispatch, "unload_interface_impl", ctypes.c_int, @@ -275,7 +279,7 @@ def unload_impl(binding: OIFPyBinding): ) -def wrap_c_function(lib, funcname, restype, argtypes): +def _wrap_c_function(lib, funcname, restype, argtypes): if isinstance(argtypes, list): if len(argtypes) == 1: assert argtypes[0] is not None, "For func(void) pass [] or None, not [None]" diff --git a/oif_impl/c/dispatch_c.c b/oif_impl/c/dispatch_c.c index b4905f1..b0a6437 100644 --- a/oif_impl/c/dispatch_c.c +++ b/oif_impl/c/dispatch_c.c @@ -17,7 +17,7 @@ typedef struct { static int IMPL_COUNTER = 0; ImplInfo * -load_backend(const char *impl_details, size_t version_major, size_t version_minor) +load_impl(const char *impl_details, size_t version_major, size_t version_minor) { // For C implementations, `impl_details` must contain the name // of the shared library with the methods implemented as functions. @@ -69,8 +69,7 @@ unload_impl(ImplInfo *impl_info_) } int -run_interface_method(ImplInfo *impl_info, const char *method, OIFArgs *in_args, - OIFArgs *out_args) +call_impl(ImplInfo *impl_info, const char *method, OIFArgs *in_args, OIFArgs *out_args) { if (impl_info->dh != OIF_LANG_C) { fprintf(stderr, "[dispatch_c] Provided implementation is not implemented in C\n"); diff --git a/oif_impl/python/dispatch_python.c b/oif_impl/python/dispatch_python.c index 0a49c3d..1286d5f 100644 --- a/oif_impl/python/dispatch_python.c +++ b/oif_impl/python/dispatch_python.c @@ -71,7 +71,7 @@ convert_oif_callback(OIFCallback *p) } ImplInfo * -load_backend(const char *impl_details, size_t version_major, size_t version_minor) +load_impl(const char *impl_details, size_t version_major, size_t version_minor) { if (Py_IsInitialized()) { fprintf(stderr, "[backend_python] Backend is already initialized\n"); @@ -179,8 +179,7 @@ load_backend(const char *impl_details, size_t version_major, size_t version_mino } int -run_interface_method(ImplInfo *impl_info, const char *method, OIFArgs *in_args, - OIFArgs *out_args) +call_impl(ImplInfo *impl_info, const char *method, OIFArgs *in_args, OIFArgs *out_args) { if (impl_info->dh != OIF_LANG_PYTHON) { fprintf(stderr, "[dispatch_python] Provided implementation is not in Python\n");