Improve dispatch API

.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,
 

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

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 <assert.h>
 #include <dlfcn.h>
-#include <ffi.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
+// This is required to avoid clang-tidy issues with `hashmap.h`.
+#if __STDC_VERSION__ < 202300L
+#define typeof __typeof__
+#endif
+
 #include <hashmap.h>
 
-#include <oif/dispatch.h>
-#include <oif/dispatch_api.h>
+#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,14 +72,19 @@ 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;
         }
     }
     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,41 +167,50 @@ 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;
     }
     else {
         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,

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,
 };

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

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);

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;
 }

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;
 }

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;
 }

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)

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)

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)