pnoltes commented on code in PR #699:
URL: https://github.com/apache/celix/pull/699#discussion_r1471676998


##########
libs/dfi/include/dyn_type.h:
##########
@@ -363,15 +361,25 @@ CELIX_DFI_EXPORT int 
dynType_typedPointer_getTypedType(dyn_type *type, dyn_type
  * @return 0 if successful.
  * @retval 1 if Cannot allocate memory.
  */
-CELIX_DFI_EXPORT int dynType_text_allocAndInit(dyn_type *type, void *textLoc, 
const char *value);
+CELIX_DFI_EXPORT int dynType_text_allocAndInit(const dyn_type* type, void* 
textLoc, const char* value);
 
 /**
  * @brief Sets the value of a simple type.
  * @param[in] type The dyn type. Must be a simple type.
  * @param[out] inst The instance of the simple type.
  * @param[in] in   The value to set.
  */
-CELIX_DFI_EXPORT void dynType_simple_setValue(dyn_type *type, void *inst, void 
*in);
+CELIX_DFI_EXPORT void dynType_simple_setValue(const dyn_type* type, void* 
inst, const void* in);
+
+/**
+ * @brief Test whether the given type is trivial.
+ * A trivial type does NOT involve any pointer type, and thus can always be 
safely shallow-copied.
+ * Any non-pointer simple type is trivial, while typed/untyped pointer is 
non-trivial.
+ * A sequence is non-trivial since it contains a pointer to the buffer.
+ * Text is nontrivial.
+ * A complex is trivial iff each field is trivial.

Review Comment:
   ```suggestion
    * A complex is trivial if each field is trivial.
   ```



##########
libs/dfi/include/dyn_interface.h:
##########
@@ -50,9 +46,8 @@ typedef struct _dyn_interface_type dyn_interface_type;
 TAILQ_HEAD(methods_head, method_entry);
 struct method_entry {
     int index;
-    char *id;
-    char *name;
-    dyn_function_type *dynFunc;
+    char* id;

Review Comment:
   It has been a while since I looked into libdfi. Is the 'id' generally the 
name or signature of a method?.
   
   I think it would be good to document (doxygen) this more explicit.



##########
libs/dfi/src/dyn_type.c:
##########
@@ -185,265 +187,200 @@ static int dynType_parseAny(FILE *stream, dyn_type 
*type) {
     return status;
 }
 
-static int dynType_parseMetaInfo(FILE *stream, dyn_type *type) {
+static int dynType_parseMetaInfo(FILE* stream, dyn_type* type) {
     int status = OK;
-    char *name = NULL;
-    char *value = NULL;
-
-    struct meta_entry *entry = calloc(1, sizeof(*entry));
-    if (entry == NULL) {
-        status = ERROR;
-    }
+    celix_autofree char* name = NULL;
+    celix_autofree char* value = NULL;
 
-    if (status == OK) {
-        status = dynCommon_parseName(stream, &name);
+    if (dynCommon_parseName(stream, &name) != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        status = dynCommon_eatChar(stream, '=');
+    if (dynCommon_eatChar(stream, '=') != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        status = dynCommon_parseName(stream, &value);
+    if (dynCommon_parseName(stream, &value) != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        status = dynCommon_eatChar(stream, ';');
+    if (dynCommon_eatChar(stream, ';') != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        entry->name = name;
-        entry->value = value;
-        TAILQ_INSERT_TAIL(&type->metaProperties, entry, entries);
-    } else {
-        LOG_ERROR("Failed to parse meta properties");
-        free(name);
-        free(value);
-        free(entry);
+    struct meta_entry *entry = calloc(1, sizeof(*entry));
+    if (entry == NULL) {
+        status = MEM_ERROR;
+        goto bail_out;
     }
+    entry->name = celix_steal_ptr(name);
+    entry->value = celix_steal_ptr(value);
+    TAILQ_INSERT_TAIL(&type->metaProperties, entry, entries);
+    return OK;
 
+bail_out:
+    celix_err_push("Failed to parse meta properties");
     return status;
 }
 
-static int dynType_parseText(FILE *stream, dyn_type *type) {
-    int status = OK;
+static int dynType_parseText(FILE* stream, dyn_type* type) {
     type->type = DYN_TYPE_TEXT;
     type->descriptor = 't';
+    type->trivial = false;
     type->ffiType = &ffi_type_pointer;
-    return status;
+    return OK;
 }
 
-static int dynType_parseEnum(FILE *stream, dyn_type *type) {
-    int status = OK;
+static int dynType_parseEnum(FILE* stream, dyn_type* type) {
     type->ffiType = &ffi_type_sint32;

Review Comment:
   IMO sint32 for a enum is good enough. But if I recall correctly the 
underlying type of a C enum is (compiler) implementation dependent. 



##########
libs/dfi/src/dyn_common.c:
##########
@@ -18,124 +18,92 @@
  */
 
 #include "dyn_common.h"
+#include "celix_err.h"
+#include "celix_stdio_cleanup.h"
+#include "celix_stdlib_cleanup.h"
 
 #include <stdlib.h>
 #include <stdio.h>
 #include <ctype.h>
 #include <stdbool.h>
 
-#if CELIX_UTILS_NO_MEMSTREAM_AVAILABLE
-#include "open_memstream.h"
-#include "fmemopen.h"
-#endif
-
 static const int OK = 0;
 static const int ERROR = 1;
 
-DFI_SETUP_LOG(dynCommon)
-
-static bool dynCommon_charIn(int c, const char *acceptedChars);
 
-int dynCommon_parseName(FILE *stream, char **result) {
+int dynCommon_parseName(FILE* stream, char** result) {
     return dynCommon_parseNameAlsoAccept(stream, NULL, result);
 }
 
-int dynCommon_parseNameAlsoAccept(FILE *stream, const char *acceptedChars, 
char **result) {
-    int status = OK;
-
-    char *buf = NULL;
+int dynCommon_parseNameAlsoAccept(FILE* stream, const char* acceptedChars, 
char** result) {
+    celix_autofree char* buf = NULL;
     size_t size = 0;
-    int strLen = 0;
-    FILE *name = open_memstream(&buf, &size);
-
-    if (name != NULL) { 
-        int c = getc(stream);
-        while (isalnum(c) || c == '_' || dynCommon_charIn(c, acceptedChars)) {
-            fputc(c, name); 
-            c = getc(stream);
-            strLen += 1;
-        }
-        fflush(name);
-        fclose(name);
-        ungetc(c, stream);
-    } else {
-        status = ERROR;
-        LOG_ERROR("Error creating mem stream for name. %s", strerror(errno));
+    celix_autoptr(FILE) name = open_memstream(&buf, &size);
+    if (name == NULL) {
+        celix_err_pushf("Error creating mem stream for name. %s", 
strerror(errno));
+        return ERROR;
     }
 
-    if (status == OK) {
-        if (strLen == 0) {
-            status = ERROR;
-            LOG_ERROR("Parsed empty name");
+    int c = getc(stream);
+    while (c != EOF && (isalnum(c) || c == '_' || (acceptedChars != NULL && 
strchr(acceptedChars, c) != NULL))) {
+        if(fputc(c, name) == EOF) {
+            celix_err_push("Error writing to mem stream for name.");
+            return ERROR;
         }
+        c = getc(stream);
+    }
+    if (c != EOF) {
+        ungetc(c, stream);
+    }
+    if(fclose(celix_steal_ptr(name)) != 0) {
+        celix_err_pushf("Error creating mem stream for name. %s", 
strerror(errno));

Review Comment:
   ```suggestion
           celix_err_pushf("Error closing mem stream for name. %s", 
strerror(errno));
   ```



##########
libs/dfi/src/dyn_type.c:
##########
@@ -185,265 +187,200 @@ static int dynType_parseAny(FILE *stream, dyn_type 
*type) {
     return status;
 }
 
-static int dynType_parseMetaInfo(FILE *stream, dyn_type *type) {
+static int dynType_parseMetaInfo(FILE* stream, dyn_type* type) {
     int status = OK;
-    char *name = NULL;
-    char *value = NULL;
-
-    struct meta_entry *entry = calloc(1, sizeof(*entry));
-    if (entry == NULL) {
-        status = ERROR;
-    }
+    celix_autofree char* name = NULL;
+    celix_autofree char* value = NULL;
 
-    if (status == OK) {
-        status = dynCommon_parseName(stream, &name);
+    if (dynCommon_parseName(stream, &name) != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        status = dynCommon_eatChar(stream, '=');
+    if (dynCommon_eatChar(stream, '=') != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        status = dynCommon_parseName(stream, &value);
+    if (dynCommon_parseName(stream, &value) != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        status = dynCommon_eatChar(stream, ';');
+    if (dynCommon_eatChar(stream, ';') != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        entry->name = name;
-        entry->value = value;
-        TAILQ_INSERT_TAIL(&type->metaProperties, entry, entries);
-    } else {
-        LOG_ERROR("Failed to parse meta properties");
-        free(name);
-        free(value);
-        free(entry);
+    struct meta_entry *entry = calloc(1, sizeof(*entry));
+    if (entry == NULL) {
+        status = MEM_ERROR;
+        goto bail_out;
     }
+    entry->name = celix_steal_ptr(name);
+    entry->value = celix_steal_ptr(value);
+    TAILQ_INSERT_TAIL(&type->metaProperties, entry, entries);
+    return OK;
 
+bail_out:
+    celix_err_push("Failed to parse meta properties");
     return status;
 }
 
-static int dynType_parseText(FILE *stream, dyn_type *type) {
-    int status = OK;
+static int dynType_parseText(FILE* stream, dyn_type* type) {
     type->type = DYN_TYPE_TEXT;
     type->descriptor = 't';
+    type->trivial = false;
     type->ffiType = &ffi_type_pointer;
-    return status;
+    return OK;
 }
 
-static int dynType_parseEnum(FILE *stream, dyn_type *type) {
-    int status = OK;
+static int dynType_parseEnum(FILE* stream, dyn_type* type) {
     type->ffiType = &ffi_type_sint32;
     type->descriptor = 'E';
+    type->trivial = true;
     type->type = DYN_TYPE_SIMPLE;
-    return status;
+    return OK;
 }
 
-static int dynType_parseComplex(FILE *stream, dyn_type *type) {
+static int dynType_parseComplex(FILE* stream, dyn_type* type) {
+    size_t nbEntries = 0;
     int status = OK;
     type->type = DYN_TYPE_COMPLEX;
     type->descriptor = '{';
+    type->trivial = true;
     type->ffiType = &type->complex.structType;
     TAILQ_INIT(&type->complex.entriesHead);
 
     int c = fgetc(stream);
-    struct complex_type_entry *entry = NULL;
+    struct complex_type_entry* entry = NULL;
     while (c != ' ' && c != '}') {
         ungetc(c,stream);
+        celix_autoptr(dyn_type) subType = NULL;
+        status = dynType_parseWithStreamOfName(stream, NULL, type, NULL, 
&subType, dynType_parseAny);
+        if (status != OK) {
+            return status;
+        }
         entry = calloc(1, sizeof(*entry));
-        if (entry != NULL) {
-            entry->type = calloc(1, sizeof(*entry->type));
+        if (entry == NULL) {
+            celix_err_push("Error allocating memory for complex_type_entry");
+            return MEM_ERROR;
         }
-        if (entry != NULL && entry->type != NULL) {
-            entry->type->parent = type;
-            entry->type->type = DYN_TYPE_INVALID;
-            TAILQ_INIT(&entry->type->nestedTypesHead);
-            TAILQ_INIT(&entry->type->metaProperties);
-            TAILQ_INSERT_TAIL(&type->complex.entriesHead, entry, entries);
-            status = dynType_parseAny(stream, entry->type);
-        } else {
-            free(entry);
-            status = MEM_ERROR;
-            LOG_ERROR("Error allocating memory for type");
+        entry->type = celix_steal_ptr(subType);
+        if (!dynType_isTrivial(entry->type)) {
+            type->trivial = false;

Review Comment:
   ```suggestion
           type->trivial = dynType_isTrivial(entry->type); 
   ```
   



##########
libs/dfi/src/dyn_common.c:
##########
@@ -18,124 +18,92 @@
  */
 
 #include "dyn_common.h"
+#include "celix_err.h"
+#include "celix_stdio_cleanup.h"
+#include "celix_stdlib_cleanup.h"
 
 #include <stdlib.h>
 #include <stdio.h>
 #include <ctype.h>
 #include <stdbool.h>
 
-#if CELIX_UTILS_NO_MEMSTREAM_AVAILABLE
-#include "open_memstream.h"
-#include "fmemopen.h"
-#endif
-
 static const int OK = 0;
 static const int ERROR = 1;
 
-DFI_SETUP_LOG(dynCommon)
-
-static bool dynCommon_charIn(int c, const char *acceptedChars);
 
-int dynCommon_parseName(FILE *stream, char **result) {
+int dynCommon_parseName(FILE* stream, char** result) {
     return dynCommon_parseNameAlsoAccept(stream, NULL, result);
 }
 
-int dynCommon_parseNameAlsoAccept(FILE *stream, const char *acceptedChars, 
char **result) {
-    int status = OK;
-
-    char *buf = NULL;
+int dynCommon_parseNameAlsoAccept(FILE* stream, const char* acceptedChars, 
char** result) {
+    celix_autofree char* buf = NULL;
     size_t size = 0;
-    int strLen = 0;
-    FILE *name = open_memstream(&buf, &size);
-
-    if (name != NULL) { 
-        int c = getc(stream);
-        while (isalnum(c) || c == '_' || dynCommon_charIn(c, acceptedChars)) {
-            fputc(c, name); 
-            c = getc(stream);
-            strLen += 1;
-        }
-        fflush(name);
-        fclose(name);
-        ungetc(c, stream);
-    } else {
-        status = ERROR;
-        LOG_ERROR("Error creating mem stream for name. %s", strerror(errno));
+    celix_autoptr(FILE) name = open_memstream(&buf, &size);
+    if (name == NULL) {
+        celix_err_pushf("Error creating mem stream for name. %s", 
strerror(errno));
+        return ERROR;
     }
 
-    if (status == OK) {
-        if (strLen == 0) {
-            status = ERROR;
-            LOG_ERROR("Parsed empty name");
+    int c = getc(stream);
+    while (c != EOF && (isalnum(c) || c == '_' || (acceptedChars != NULL && 
strchr(acceptedChars, c) != NULL))) {
+        if(fputc(c, name) == EOF) {
+            celix_err_push("Error writing to mem stream for name.");
+            return ERROR;
         }
+        c = getc(stream);
+    }
+    if (c != EOF) {
+        ungetc(c, stream);
+    }
+    if(fclose(celix_steal_ptr(name)) != 0) {
+        celix_err_pushf("Error creating mem stream for name. %s", 
strerror(errno));
+        return ERROR;
     }
 
-    if (status == OK) {
-       *result = buf;
-    } else if (buf != NULL) {
-        free(buf);
+    if (size == 0) {
+        celix_err_push("Parsed empty name");
+        return ERROR;
     }
 
-    return status;
+    *result = celix_steal_ptr(buf);
+
+    return OK;
 }
 
-int dynCommon_parseNameValue(FILE *stream, char **outName, char **outValue) {
+static int dynCommon_parseNameValue(FILE* stream, char** outName, char** 
outValue) {
     int status;
-    char *name = NULL;
-    char *value = NULL;
-
-    status = dynCommon_parseName(stream, &name);
-    if (status == OK) {
-        status = dynCommon_eatChar(stream, '=');
-    }
-    if (status == OK) {
-        const char *valueAcceptedChars = ".<>{}[]?;:~!@#$%^&*()_+-=,./\\'\"";
-
-        status = dynCommon_parseNameAlsoAccept(stream, valueAcceptedChars, 
&value); //NOTE use different more lenient function e.g. only stop at '\n' ?
-    }
-
-    if (status == OK) {
-        *outName = name;
-        *outValue = value;
-    } else {
-        if (name != NULL) {
-            free(name);
+    celix_autofree char* name = NULL;
+    celix_autofree char* value = NULL;
+    do {
+        if ((status = dynCommon_parseName(stream, &name)) != OK) {
+            break;

Review Comment:
   why a do/while with breaks on error and not an if with an early return?



##########
libs/dfi/src/dyn_descriptor.c:
##########
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "dyn_descriptor.h"
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
+
+#include <assert.h>
+
+static const int OK = 0;
+static const int ERROR = 1;
+
+static int celix_dynDescriptor_parseTypes(celix_descriptor_t* descriptor, FILE 
*stream) {
+    int status;
+
+    //expected input (Name)=<Type>\n
+    int peek = fgetc(stream);
+    while (peek != ':' && peek != EOF) {
+        ungetc(peek, stream);
+
+        celix_autofree char* name = NULL;
+        if ((status = dynCommon_parseName(stream, &name)) != OK) {
+            return status;
+        }
+
+        if ((status = dynCommon_eatChar(stream, '=')) != OK) {
+            return status;
+        }
+
+        celix_autoptr(dyn_type) type = NULL;
+        if ((status = dynType_parseOfName(stream, celix_steal_ptr(name), 
&descriptor->types, &type)) != OK) {
+            return status;
+        }
+        if ((status = dynCommon_eatChar(stream, '\n')) != OK) {
+            return status;
+        }
+
+        struct type_entry *entry = NULL;
+        entry = calloc(1, sizeof(*entry));
+        if (entry == NULL) {
+            celix_err_pushf("Error allocating memory for type entry");
+            return ERROR;
+        }
+        entry->type = celix_steal_ptr(type);
+        TAILQ_INSERT_TAIL(&descriptor->types, entry, entries);
+
+        peek = fgetc(stream);
+    }
+    if (peek != EOF) {
+        ungetc(peek, stream);
+    }
+
+    return OK;
+}
+
+static int celix_dynDescriptor_parseSection(celix_descriptor_t* descriptor, 
FILE *stream,
+                                            int 
(*parseSection)(celix_descriptor_t* descriptor, const char* secName, FILE 
*stream)) {
+    int status = OK;
+    celix_autofree char *sectionName = NULL;
+
+    if ((status = dynCommon_eatChar(stream, ':')) != OK) {
+        return status;
+    }
+
+    if ((status = dynCommon_parseName(stream, &sectionName)) != OK) {
+        return status;
+    }
+
+    if ((status = dynCommon_eatChar(stream, '\n')) != OK) {
+        return status;
+    }
+
+    if (strcmp("header", sectionName) == 0) {
+        status = dynCommon_parseNameValueSection(stream, &descriptor->header);
+    } else if (strcmp("annotations", sectionName) == 0) {
+        status = dynCommon_parseNameValueSection(stream, 
&descriptor->annotations);
+    } else if (strcmp("types", sectionName) == 0) {
+        status = celix_dynDescriptor_parseTypes(descriptor, stream);
+    } else {
+        status = parseSection(descriptor, sectionName, stream);
+    }
+
+    return status;
+}
+
+static int celix_dynDescriptor_checkInterface(celix_descriptor_t* descriptor) {
+    //check header section
+    bool foundType = false;
+    bool foundVersion = false;
+    bool foundName = false;
+    struct namval_entry *entry = NULL;
+    TAILQ_FOREACH(entry, &descriptor->header, entries) {
+        if (strcmp(entry->name, "type") == 0) {
+            foundType = true;
+        } else if (strcmp(entry->name, "version") == 0) {
+            foundVersion = true;
+        } else if (strcmp(entry->name, "name") == 0) {
+            foundName = true;
+        }
+    }
+
+    if (!foundType || !foundVersion || !foundName) {
+        celix_err_pushf("Parse Error. There must be a header section with a 
type, version and name entry");

Review Comment:
   👍 



##########
libs/dfi/src/dyn_type.c:
##########
@@ -185,265 +187,200 @@ static int dynType_parseAny(FILE *stream, dyn_type 
*type) {
     return status;
 }
 
-static int dynType_parseMetaInfo(FILE *stream, dyn_type *type) {
+static int dynType_parseMetaInfo(FILE* stream, dyn_type* type) {
     int status = OK;
-    char *name = NULL;
-    char *value = NULL;
-
-    struct meta_entry *entry = calloc(1, sizeof(*entry));
-    if (entry == NULL) {
-        status = ERROR;
-    }
+    celix_autofree char* name = NULL;
+    celix_autofree char* value = NULL;
 
-    if (status == OK) {
-        status = dynCommon_parseName(stream, &name);
+    if (dynCommon_parseName(stream, &name) != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;

Review Comment:
   goto is needed to reuse the celix_err_push(msg) + return status?



##########
libs/dfi/src/dyn_common.c:
##########
@@ -18,124 +18,92 @@
  */
 
 #include "dyn_common.h"
+#include "celix_err.h"
+#include "celix_stdio_cleanup.h"
+#include "celix_stdlib_cleanup.h"
 
 #include <stdlib.h>
 #include <stdio.h>
 #include <ctype.h>
 #include <stdbool.h>
 
-#if CELIX_UTILS_NO_MEMSTREAM_AVAILABLE
-#include "open_memstream.h"
-#include "fmemopen.h"
-#endif
-
 static const int OK = 0;
 static const int ERROR = 1;
 
-DFI_SETUP_LOG(dynCommon)
-
-static bool dynCommon_charIn(int c, const char *acceptedChars);
 
-int dynCommon_parseName(FILE *stream, char **result) {
+int dynCommon_parseName(FILE* stream, char** result) {

Review Comment:
   Given that celix_status_t now support a custom facility error part, I think 
the int return can be upgraded to a celix_status_t. And because celix_status_t 
is a typedef to int, this maybe is even backwards compatible. 
   
   But this would be nice as a future enhancement and port of this pull 
request. 



##########
libs/dfi/src/dyn_type.c:
##########
@@ -452,64 +389,60 @@ static int dynType_parseSequence(FILE *stream, dyn_type 
*type) {
     type->sequence.seqType.size = 0;
     type->sequence.seqType.alignment = 0;
 
-    status = dynType_parseWithStream(stream, NULL, type, NULL, 
&type->sequence.itemType);
-
-    if (status == OK) {
-        type->ffiType = &type->sequence.seqType;
-        dynType_prepCif(&type->sequence.seqType);
+    status = dynType_parseWithStreamOfName(stream, NULL, type, NULL, 
&type->sequence.itemType, dynType_parseAny);
+    if (status != OK) {
+        return status;
     }
 
-    return status;
+    type->ffiType = &type->sequence.seqType;
+    (void)ffi_get_struct_offsets(FFI_DEFAULT_ABI, type->ffiType, NULL);

Review Comment:
   Nice. Previously this was not done.. was this not needed in most case due to 
an eventual call to `ffi_prep_cif`?



##########
libs/dfi/src/dyn_type.c:
##########
@@ -185,265 +187,200 @@ static int dynType_parseAny(FILE *stream, dyn_type 
*type) {
     return status;
 }
 
-static int dynType_parseMetaInfo(FILE *stream, dyn_type *type) {
+static int dynType_parseMetaInfo(FILE* stream, dyn_type* type) {
     int status = OK;
-    char *name = NULL;
-    char *value = NULL;
-
-    struct meta_entry *entry = calloc(1, sizeof(*entry));
-    if (entry == NULL) {
-        status = ERROR;
-    }
+    celix_autofree char* name = NULL;
+    celix_autofree char* value = NULL;
 
-    if (status == OK) {
-        status = dynCommon_parseName(stream, &name);
+    if (dynCommon_parseName(stream, &name) != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        status = dynCommon_eatChar(stream, '=');
+    if (dynCommon_eatChar(stream, '=') != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        status = dynCommon_parseName(stream, &value);
+    if (dynCommon_parseName(stream, &value) != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        status = dynCommon_eatChar(stream, ';');
+    if (dynCommon_eatChar(stream, ';') != OK) {
+        status = PARSE_ERROR;
+        goto bail_out;
     }
-
-    if (status == OK) {
-        entry->name = name;
-        entry->value = value;
-        TAILQ_INSERT_TAIL(&type->metaProperties, entry, entries);
-    } else {
-        LOG_ERROR("Failed to parse meta properties");
-        free(name);
-        free(value);
-        free(entry);
+    struct meta_entry *entry = calloc(1, sizeof(*entry));
+    if (entry == NULL) {
+        status = MEM_ERROR;
+        goto bail_out;
     }
+    entry->name = celix_steal_ptr(name);
+    entry->value = celix_steal_ptr(value);
+    TAILQ_INSERT_TAIL(&type->metaProperties, entry, entries);
+    return OK;
 
+bail_out:
+    celix_err_push("Failed to parse meta properties");
     return status;
 }
 
-static int dynType_parseText(FILE *stream, dyn_type *type) {
-    int status = OK;
+static int dynType_parseText(FILE* stream, dyn_type* type) {
     type->type = DYN_TYPE_TEXT;
     type->descriptor = 't';
+    type->trivial = false;
     type->ffiType = &ffi_type_pointer;
-    return status;
+    return OK;
 }
 
-static int dynType_parseEnum(FILE *stream, dyn_type *type) {
-    int status = OK;
+static int dynType_parseEnum(FILE* stream, dyn_type* type) {
     type->ffiType = &ffi_type_sint32;
     type->descriptor = 'E';
+    type->trivial = true;
     type->type = DYN_TYPE_SIMPLE;
-    return status;
+    return OK;
 }
 
-static int dynType_parseComplex(FILE *stream, dyn_type *type) {
+static int dynType_parseComplex(FILE* stream, dyn_type* type) {
+    size_t nbEntries = 0;
     int status = OK;
     type->type = DYN_TYPE_COMPLEX;
     type->descriptor = '{';
+    type->trivial = true;

Review Comment:
   nitpick: not needed, will be set in line 266



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to