Code cleanup and testing coverage improvement of dynFunction.

1. Apply early return to dynFunction_parseDescriptor.
2. Release resources in strict reverse order in dynFunction_destroy.
3. Improve error handling of dynFunction_createClosure.
4. More test cases to improve coverage.

libs/dfi/error_injector/ffi/CMakeLists.txt

@@ -22,5 +22,7 @@ target_link_libraries(ffi_ei PUBLIC Celix::error_injector libffi::libffi)
 
 target_link_options(ffi_ei INTERFACE
         LINKER:--wrap,ffi_prep_cif
+        LINKER:--wrap,ffi_closure_alloc
+        LINKER:--wrap,ffi_prep_closure_loc
 )
 add_library(Celix::ffi_ei ALIAS ffi_ei)

libs/dfi/error_injector/ffi/include/ffi_ei.h

@@ -27,6 +27,10 @@ extern "C" {
 
 CELIX_EI_DECLARE(ffi_prep_cif, ffi_status);
 
+CELIX_EI_DECLARE(ffi_closure_alloc, void*);
+
+CELIX_EI_DECLARE(ffi_prep_closure_loc, ffi_status);
+
 #ifdef __cplusplus
 }
 #endif

libs/dfi/error_injector/ffi/src/ffi_ei.cc

@@ -26,4 +26,19 @@ ffi_status __wrap_ffi_prep_cif(ffi_cif *cif, ffi_abi abi, unsigned int nargs, ff
     CELIX_EI_IMPL(ffi_prep_cif);
     return __real_ffi_prep_cif(cif, abi, nargs, rtype, atypes);
 }
+
+void *__real_ffi_closure_alloc(size_t size, void **code);
+CELIX_EI_DEFINE(ffi_closure_alloc, void*)
+void *__wrap_ffi_closure_alloc(size_t size, void **code) {
+    CELIX_EI_IMPL(ffi_closure_alloc);
+    return __real_ffi_closure_alloc(size, code);
+}
+
+ffi_status __real_ffi_prep_closure_loc(ffi_closure *closure, ffi_cif *cif, void (*fun)(ffi_cif *, void *, void **, void *), void *user_data, void *codeloc);
+CELIX_EI_DEFINE(ffi_prep_closure_loc, ffi_status)
+ffi_status __wrap_ffi_prep_closure_loc(ffi_closure *closure, ffi_cif *cif, void (*fun)(ffi_cif *, void *, void **, void *), void *user_data, void *codeloc) {
+    CELIX_EI_IMPL(ffi_prep_closure_loc);
+    return __real_ffi_prep_closure_loc(closure, cif, fun, user_data, codeloc);
+}
+
 }

libs/dfi/gtest/CMakeLists.txt

@@ -52,6 +52,7 @@ if (EI_TESTS)
 			Celix::stdio_ei
 			Celix::string_ei
 			Celix::ffi_ei
+			Celix::asprintf_ei
 			GTest::gtest GTest::gtest_main
 	)
 	add_test(NAME run_test_dfi_with_ei COMMAND test_dfi_with_ei)

libs/dfi/gtest/src/dyn_function_ei_tests.cc

@@ -21,6 +21,7 @@
 #include "dyn_example_functions.h"
 
 #include "celix_err.h"
+#include "asprintf_ei.h"
 #include "ffi_ei.h"
 #include "malloc_ei.h"
 #include "stdio_ei.h"
@@ -34,6 +35,7 @@ class DynFunctionErrorInjectionTestSuite : public ::testing::Test {
 public:
     DynFunctionErrorInjectionTestSuite() = default;
     ~DynFunctionErrorInjectionTestSuite() override {
+        celix_ei_expect_asprintf(nullptr, 0, -1);
         celix_ei_expect_fmemopen(nullptr, 0, nullptr);
         celix_ei_expect_ffi_prep_cif(nullptr, 0, FFI_OK);
         celix_ei_expect_calloc(nullptr, 0, nullptr);
@@ -65,4 +67,53 @@ TEST_F(DynFunctionErrorInjectionTestSuite, ParseError) {
     std::string result = "Error creating mem stream for descriptor string. ";
     result += strerror(ENOMEM);
     EXPECT_STREQ(result.c_str(), celix_err_popLastError());
+
+    celix_ei_expect_asprintf((void*) dynFunction_parse, 1, -1, 3);
+    rc = dynFunction_parseWithStr(EXAMPLE1_DESCRIPTOR, nullptr, &dynFunc);
+    EXPECT_NE(0, rc);
+    EXPECT_STREQ("Error parsing descriptor", celix_err_popLastError());
+    EXPECT_STREQ("Error allocating argument name", celix_err_popLastError());
+
+
+    celix_ei_expect_calloc((void*)dynFunction_parse, 1, nullptr, 3);
+    rc = dynFunction_parseWithStr(EXAMPLE1_DESCRIPTOR, nullptr, &dynFunc);
+    EXPECT_NE(0, rc);
+    EXPECT_STREQ("Error parsing descriptor", celix_err_popLastError());
+    EXPECT_STREQ("Error allocating arg", celix_err_popLastError());
+}
+
+class DynClosureErrorInjectionTestSuite : public ::testing::Test {
+public:
+    DynClosureErrorInjectionTestSuite() = default;
+    ~DynClosureErrorInjectionTestSuite() override {
+        celix_ei_expect_ffi_prep_closure_loc(nullptr, 0, FFI_OK);
+        celix_ei_expect_ffi_closure_alloc(nullptr, 0, nullptr);
+    }
+};
+
+
+static void example1_binding(void*, void* args[], void *out) {
+    int32_t a = *((int32_t *)args[0]);
+    int32_t b = *((int32_t *)args[1]);
+    int32_t c = *((int32_t *)args[2]);
+    int32_t *ret = (int32_t *)out;
+    *ret = a + b + c;
+}
+
+TEST_F(DynClosureErrorInjectionTestSuite, CreatError) {
+    celix_autoptr(dyn_function_type) dynFunc = nullptr;
+    int rc;
+    rc = dynFunction_parseWithStr(EXAMPLE1_DESCRIPTOR, nullptr, &dynFunc);
+    ASSERT_EQ(0, rc);
+
+    int32_t (*func)(int32_t a, int32_t b, int32_t c) = NULL;
+    celix_ei_expect_ffi_closure_alloc((void*)dynFunction_createClosure, 0, nullptr);
+    rc = dynFunction_createClosure(dynFunc, example1_binding, NULL, (void(**)(void))&func);
+    ASSERT_NE(0, rc);
+    EXPECT_NE(0, dynFunction_getFnPointer(dynFunc, (void(**)(void))&func));
+
+    celix_ei_expect_ffi_prep_closure_loc((void*)dynFunction_createClosure, 0, FFI_BAD_ABI);
+    rc = dynFunction_createClosure(dynFunc, example1_binding, NULL, (void(**)(void))&func);
+    ASSERT_NE(0, rc);
+    EXPECT_NE(0, dynFunction_getFnPointer(dynFunc, (void(**)(void))&func));
 }

libs/dfi/gtest/src/dyn_function_tests.cpp

@@ -40,37 +40,29 @@ class DynFunctionTests : public ::testing::Test {
 
 };
 
+TEST_F(DynFunctionTests, DynFuncTest1) {
+    dyn_function_type *dynFunc = nullptr;
+    int rc;
+    void (*fp)(void) = (void (*)(void)) example1;
 
-extern "C" {
-    static bool func_test1() {
-        dyn_function_type *dynFunc = nullptr;
-        int rc;
-        void (*fp)(void) = (void (*)(void)) example1;
-
-        rc = dynFunction_parseWithStr(EXAMPLE1_DESCRIPTOR, nullptr, &dynFunc);
-
-        ffi_sarg rVal = 0;
-        int32_t a = 2;
-        int32_t b = 4;
-        int32_t c = 8;
-        void *values[3];
-        values[0] = &a;
-        values[1] = &b;
-        values[2] = &c;
+    rc = dynFunction_parseWithStr(EXAMPLE1_DESCRIPTOR, nullptr, &dynFunc);
+    ASSERT_EQ(0, rc);
+    EXPECT_TRUE(dynFunction_hasReturn(dynFunc));
 
-        if (rc == 0) {
-            rc = dynFunction_call(dynFunc, fp, &rVal, values);
-            dynFunction_destroy(dynFunc);
-        }
+    ffi_sarg rVal = 0;
+    int32_t a = 2;
+    int32_t b = 4;
+    int32_t c = 8;
+    void *values[3];
+    values[0] = &a;
+    values[1] = &b;
+    values[2] = &c;
 
-        return rc == 0 && rVal == 14;
-    }
-}
+    rc = dynFunction_call(dynFunc, fp, &rVal, values);
+    dynFunction_destroy(dynFunc);
 
-TEST_F(DynFunctionTests, DynFuncTest1) {
-    //NOTE only using libffi with extern C, because combining libffi with EXPECT_*/ASSERT_* call leads to
-    //corrupted memory. Note that libffi is a function for interfacing with C not C++
-    EXPECT_TRUE(func_test1());
+    EXPECT_EQ(0, rc);
+    EXPECT_EQ(14, rVal);
 }
 
 extern "C" {
@@ -190,13 +182,14 @@ TEST_F(DynFunctionTests, DynFuncTest3) {
     EXPECT_TRUE(func_test3());
 }
 
-extern "C" {
-static bool func_test4() {
+TEST_F(DynFunctionTests, DynFuncTest4) {
     dyn_function_type *dynFunc = nullptr;
     void (*fp)(void) = (void(*)(void)) example4Func;
     int rc;
 
     rc = dynFunction_parseWithStr(EXAMPLE4_DESCRIPTOR, nullptr, &dynFunc);
+    ASSERT_EQ(0, rc);
+    EXPECT_FALSE(dynFunction_hasReturn(dynFunc));
 
     double buf[4];
     buf[0] = 1.1;
@@ -208,19 +201,9 @@ static bool func_test4() {
 
     void *args[1];
     args[0] = &seq;
-    if (rc == 0) {
-        rc = dynFunction_call(dynFunc, fp, nullptr, args);
-        dynFunction_destroy(dynFunc);
-    }
-
-    return rc == 0;
-}
-}
-
-TEST_F(DynFunctionTests, DynFuncTest4) {
-    //NOTE only using libffi with extern C, because combining libffi with EXPECT_*/ASSERT_* call leads to
-    //corrupted memory. Note that libffi is a function for interfacing with C not C++
-    EXPECT_TRUE(func_test4());
+    rc = dynFunction_call(dynFunc, fp, nullptr, args);
+    dynFunction_destroy(dynFunc);
+    EXPECT_EQ(0, rc);
 }
 
 extern "C" {
@@ -260,12 +243,24 @@ TEST_F(DynFunctionTests, InvalidDynFuncTest) {
     int rc1 = dynFunction_parseWithStr(INVALID_FUNC_DESCRIPTOR, nullptr, &dynFunc);
     EXPECT_NE(0, rc1);
     EXPECT_STREQ("Error parsing descriptor", celix_err_popLastError());
-    EXPECT_STREQ("Expected '(' token got '$'", celix_err_popLastError());
+    EXPECT_STREQ("Error parsing, expected token '(' got '$' at position 8", celix_err_popLastError());
 
     dynFunc = nullptr;
     int rc2 = dynFunction_parseWithStr(INVALID_FUNC_TYPE_DESCRIPTOR, nullptr, &dynFunc);
     EXPECT_NE(0, rc2);
     EXPECT_STREQ("Error parsing descriptor", celix_err_popLastError());
     EXPECT_STREQ("Error unsupported type 'H'", celix_err_popLastError());
+
+    dynFunc = nullptr;
+    int rc3 = dynFunction_parseWithStr("$xample(III)I", nullptr, &dynFunc);
+    EXPECT_NE(0, rc3);
+    EXPECT_STREQ("Error parsing descriptor", celix_err_popLastError());
+    EXPECT_STREQ("Parsed empty name", celix_err_popLastError());
+
+    dynFunc = nullptr;
+    int rc4 = dynFunction_parseWithStr("example(III", nullptr, &dynFunc);
+    EXPECT_NE(0, rc4);
+    EXPECT_STREQ("Error parsing descriptor", celix_err_popLastError());
+    EXPECT_STREQ("Error missing ')'", celix_err_popLastError());
 }