PengZheng commented on code in PR #765:
URL: https://github.com/apache/celix/pull/765#discussion_r1696446011


##########
libs/utils/include/celix_err.h:
##########
@@ -94,6 +94,28 @@ CELIX_UTILS_EXPORT void celix_err_printErrors(FILE* stream, 
const char* prefix,
  */
 CELIX_UTILS_EXPORT int celix_err_dump(char* buf, size_t size, const char* 
prefix, const char* postfix);
 
+/*!

Review Comment:
   I think the two macros, which represent a very specific error handling 
style,  does not belong to `celix_err.h`. Moreover, including them makes 
`celix_err.h` not self-contained any longer (missing `errno.h`).
   
   Or we include `celix_errno.h`, or we have a dedicate header including all 
these error handling macros.
   
   WDYT?



##########
libs/utils/include/celix_err.h:
##########
@@ -94,6 +94,28 @@ CELIX_UTILS_EXPORT void celix_err_printErrors(FILE* stream, 
const char* prefix,
  */
 CELIX_UTILS_EXPORT int celix_err_dump(char* buf, size_t size, const char* 
prefix, const char* postfix);
 
+/*!
+ * Helper macro that returns with ENOMEM if the status is ENOMEM and logs an 
"<func>: Out of memory" error to celix_err.
+ */
+#define CELIX_RETURN_IF_ENOMEM(status)                                         
                                        \
+    do {                                                                       
                                        \
+        if ((status) == ENOMEM) {                                              
                                        \
+            celix_err_pushf("%s: Out of memory", __func__);                    
                                        \
+            return status;                                                     
                                        \
+        }                                                                      
                                        \
+    } while (0)
+
+/*!
+ * Helper macro that returns with ENOMEM if the arg is NULL and logs an 
"<func>: Out of memory" error to celix_err.
+ */
+#define CELIX_RETURN_IF_NULL(arg)                                              
                                        \
+    do {                                                                       
                                        \
+        if ((arg) == NULL) {                                                   
                                        \
+            celix_err_pushf("%s: Out of memory", __func__);                    
                                        \
+            return status;                                                     
                                        \

Review Comment:
   ```suggestion
               return ENOMEM;                                                   
                                          \
   ```
   
   Otherwise, all usages of this macros are incorrect.



##########
libs/framework/src/bundle_archive.h:
##########
@@ -56,36 +56,36 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t 
bundleArchive_getId(bundle_arch
 CELIX_FRAMEWORK_EXPORT celix_status_t 
bundleArchive_getLocation(bundle_archive_pt archive, const char **location)
     __attribute__((deprecated("use celix_bundle_getLocation instead")));

Review Comment:
   Unneeded attribute.



##########
libs/framework/src/bundle_archive.c:
##########
@@ -229,27 +230,26 @@ static celix_status_t 
celix_bundleArchive_createCacheDirectory(bundle_archive_pt
     }
 
     //populate bundle symbolic name and version from manifest
-    archive->bundleSymbolicName = 
celix_utils_strdup(manifest_getValue(*manifestOut, 
CELIX_FRAMEWORK_BUNDLE_SYMBOLICNAME));
+    archive->bundleSymbolicName = 
celix_utils_strdup(celix_bundleManifest_getBundleSymbolicName(manifest));

Review Comment:
   From what I understand, every archive contains a valid revision, which in 
turn contains a manifest.  Thus there is no need to duplicate information 
within manifest.



##########
cmake/cmake_celix/templates/MANIFEST.json.in:
##########
@@ -0,0 +1,14 @@
+{
+    $<JOIN:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_HEADERS>,$<COMMA>
+    
>$<$<BOOL:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_HEADERS>>:$<COMMA>>
+    "CELIX_BUNDLE_MANIFEST_VERSION" : "2.0.0",
+    "CELIX_BUNDLE_SYMBOLIC_NAME" : 
"$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_SYMBOLIC_NAME>",
+    "CELIX_BUNDLE_VERSION" : 
"$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_VERSION>",

Review Comment:
   To make strict typing work, this need to be a version string 
(`celix_properties_isVersionString`). 



##########
libs/framework/src/module.c:
##########
@@ -44,183 +45,147 @@
 #define CELIX_LIBRARY_EXTENSION = ".dll";
 #endif
 
-struct module {
+struct celix_module {
     celix_framework_t* fw;
-
-    celix_version_t* version;
-    char* symbolicName;
-    char* group;
-    char* name;
-    char* description;
     bool resolved;
-
-    char* id;
-
     celix_bundle_t* bundle;
-
-    celix_thread_mutex_t handlesLock; // protects libraryHandles and 
bundleActivatorHandle
+    celix_thread_mutex_t handlesLock; // protects libraryHandles
     celix_array_list_t* libraryHandles;
-    void* bundleActivatorHandle;
 };
 
-module_pt module_create(manifest_pt headerMap, const char * moduleId, 
bundle_pt bundle) {
-    module_pt module = NULL;
-    manifest_parser_pt mp;
-    celix_framework_t* fw = NULL;
-    bundle_getFramework(bundle, &fw);
-
-    if (headerMap != NULL && fw != NULL) {
-        module = (module_pt) calloc(1, sizeof(*module));
-        module->fw = fw;
-        module->id = strdup(moduleId);
-        module->bundle = bundle;
-        module->resolved = false;
-        celixThreadMutex_create(&module->handlesLock, NULL);
-        module->libraryHandles = celix_arrayList_create();
+static celix_bundle_manifest_t* 
celix_module_getManifestFromBundle(celix_bundle_t* bundle) {
+    bundle_archive_t* archive = celix_bundle_getArchive(bundle);
+    celix_bundle_revision_t* revision = NULL;
+    (void)bundleArchive_getCurrentRevision(archive, &revision);
+    return celix_bundleRevision_getManifest(revision);
+}
 
+celix_module_t* module_create(celix_bundle_t* bundle) {
+    assert(bundle != NULL);
 
-        if (manifestParser_create(module, headerMap, &mp) == CELIX_SUCCESS) {
-            module->symbolicName = NULL;
-            manifestParser_getAndDuplicateSymbolicName(mp, 
&module->symbolicName);
+    celix_framework_t* fw;
+    bundle_getFramework(bundle, &fw);
 
-            module->group = NULL;
-            manifestParser_getAndDuplicateGroup(mp, &module->group);
+    celix_autoptr(celix_module_t) module = calloc(1, sizeof(*module));
+    celix_autoptr(celix_array_list_t) libraryHandles = 
celix_arrayList_createPointerArray();
+    if (!module || !libraryHandles) {
+        fw_log(fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to create module, 
out of memory");
+        return NULL;
+    }
 
-            module->name = NULL;
-            manifestParser_getAndDuplicateName(mp, &module->name);
+    celix_status_t status = celixThreadMutex_create(&module->handlesLock, 
NULL);
+    if (status != CELIX_SUCCESS) {
+        fw_log(fw->logger,
+               CELIX_LOG_LEVEL_ERROR,
+               "Failed to create module, error creating mutex: %s",
+               celix_strerror(errno));
+        celix_framework_logTssErrors(fw->logger, CELIX_LOG_LEVEL_ERROR);
+        return NULL;
+    }
 
-            module->description = NULL;
-            manifestParser_getAndDuplicateDescription(mp, 
&module->description);
+    module->fw = fw;
+    module->bundle = bundle;
+    module->resolved = false;
+    module->libraryHandles = celix_steal_ptr(libraryHandles);
 
-            module->version = NULL;
-            manifestParser_getBundleVersion(mp, &module->version);
+    return celix_steal_ptr(module);
+}
 
-            manifestParser_destroy(mp);
-        }
+celix_module_t* module_createFrameworkModule(celix_framework_t* fw, bundle_pt 
bundle) {

Review Comment:
   It seems identical to `module_create` so that we can combine them into one.



##########
libs/framework/src/bundle.c:
##########
@@ -495,23 +362,23 @@ char* celix_bundle_getDataFile(const celix_bundle_t* bnd, 
const char *path) {
 }
 
 const char* celix_bundle_getManifestValue(const celix_bundle_t* bnd, const 
char* attribute) {
-       const char* header = NULL;
-       if (bnd != NULL) {
+    const char* header = NULL;
+    if (bnd != NULL) {
         bundle_archive_t* arch = NULL;
         bundle_getArchive(bnd, &arch);
         if (arch != NULL) {
-            bundle_revision_t* rev = NULL;
+            celix_bundle_revision_t* rev = NULL;
             bundleArchive_getCurrentRevision(arch, &rev);
             if (rev != NULL) {
-                manifest_pt man = NULL;
-                bundleRevision_getManifest(rev, &man);
-                if (man != NULL ) {
-                    header = manifest_getValue(man, attribute);
+                celix_bundle_manifest_t* man = 
celix_bundleRevision_getManifest(rev);

Review Comment:
   I note there is already `celix_module_getManifestFromBundle`. Why not 
`celix_bundle_getManifest` to remove all these duplications?



##########
libs/utils/include/celix_properties.h:
##########
@@ -450,7 +450,8 @@ CELIX_UTILS_EXPORT const celix_version_t* 
celix_properties_getVersion(const celi
  * @brief Get a value of a property as a copied Celix version.
  *
  * If the property value is a Celix version, a copy of the found version will 
be returned.
- * If the property value is a string, this function will attempt to convert it 
to a new Celix version.
+ * If the property value is present, but not a Celix version, this function 
will attempt to convert the property value

Review Comment:
   We need a test case for non-string value type.



##########
libs/framework/include_deprecated/bundle_context.h:
##########
@@ -26,22 +26,22 @@
  * Framework.
  */
 
-#include "celix_types.h"
+#include <stdlib.h>
 
+#include "bundle_context.h"

Review Comment:
   Why include itself?



##########
libs/framework/src/bundle.c:
##########
@@ -495,23 +362,23 @@ char* celix_bundle_getDataFile(const celix_bundle_t* bnd, 
const char *path) {
 }
 
 const char* celix_bundle_getManifestValue(const celix_bundle_t* bnd, const 
char* attribute) {
-       const char* header = NULL;
-       if (bnd != NULL) {
+    const char* header = NULL;
+    if (bnd != NULL) {
         bundle_archive_t* arch = NULL;
         bundle_getArchive(bnd, &arch);
         if (arch != NULL) {

Review Comment:
   I think we can not have a bundle without an archive. Similarly there are no 
archive without revision and no revision without a manifest. Thus we can get 
rid of these checks. Of courese this can be left for a further PR.



##########
libs/framework/src/bundle_revision.c:
##########
@@ -24,9 +24,17 @@
 #include "bundle_revision_private.h"
 #include "framework_private.h"
 
-celix_status_t celix_bundleRevision_create(celix_framework_t* fw, const char 
*root, const char *location, manifest_pt manifest, bundle_revision_pt 
*bundle_revision) {
+struct celix_bundle_revision {
+    celix_framework_t* fw;
+    char* root;
+    char* location;
+    celix_bundle_manifest_t* manifest;

Review Comment:
   I think revision can be removed now, since it does not contain any 
information that archive does not already have once manifest is moved to 
archive.



##########
libs/framework/src/bundle.c:
##########
@@ -225,63 +198,7 @@ celix_status_t bundle_createModule(bundle_pt bundle, 
module_pt* moduleOut) {
     return status;
 }
 
-celix_status_t bundle_start(celix_bundle_t* bundle) {
-    //note deprecated call use celix_bundleContext_startBundle instead
-    return celix_framework_startBundle(bundle->framework, 
celix_bundle_getId(bundle));
-}
-
-celix_status_t bundle_update(bundle_pt bundle, const char* updatedBundleUrl) {
-    return celix_framework_updateBundle(bundle->framework, 
celix_bundle_getId(bundle), updatedBundleUrl);
-}
-
-celix_status_t bundle_stop(bundle_pt bundle) {
-    //note deprecated call use celix_bundleContext_stopBundle instead
-    return celix_framework_stopBundle(bundle->framework, 
celix_bundle_getId(bundle));
-}
-
-celix_status_t bundle_uninstall(bundle_pt bundle) {
-    //note deprecated call use celix_bundleContext_uninstallBundle instead
-    return celix_framework_uninstallBundle(bundle->framework, 
celix_bundle_getId(bundle));
-}
-
-celix_status_t bundle_setPersistentStateInactive(bundle_pt bundle) {
-       celix_status_t status;
-       bool systemBundle;
-
-       status = bundle_isSystemBundle(bundle, &systemBundle);
-       if (status == CELIX_SUCCESS) {
-               if (!systemBundle) {
-                       status = 
bundleArchive_setPersistentState(bundle->archive, CELIX_BUNDLE_STATE_INSTALLED);
-               }
-       }
-
-       framework_logIfError(bundle->framework->logger, status, NULL, "Failed 
to set persistent state to inactive");
-
-       return status;
-}
-
-celix_status_t bundle_setPersistentStateUninstalled(bundle_pt bundle) {
-       celix_status_t status;
-       bool systemBundle;
-
-       status = bundle_isSystemBundle(bundle, &systemBundle);
-       if (status == CELIX_SUCCESS) {
-               if (!systemBundle) {
-                       status = 
bundleArchive_setPersistentState(bundle->archive, 
CELIX_BUNDLE_STATE_UNINSTALLED);
-               }
-       }
-
-       framework_logIfError(bundle->framework->logger, status, NULL, "Failed 
to set persistent state to uninstalled");
-
-    return status;
-}
-
-celix_status_t bundle_revise(bundle_pt bundle, const char * location, const 
char *inputFile) {
-    fw_log(bundle->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Usage of 
bundle_revise is deprecated and no longer needed. Called for bundle %s", 
bundle->symbolicName);
-    return CELIX_SUCCESS;
-}
-
-celix_status_t bundle_addModule(bundle_pt bundle, module_pt module) {

Review Comment:
   I noted that there are still of unnecessary copies of 
`symbolicName`/`name`/`group`/`description` left in `bundle`. Shall we use the 
single copy in the manifest instead?



##########
libs/framework/src/celix_bundle_manifest.c:
##########
@@ -0,0 +1,284 @@
+/*
+ * 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 "celix_bundle_manifest.h"
+#include <ctype.h>
+#include <string.h>
+
+#include "celix_err.h"
+#include "celix_properties.h"
+#include "celix_stdlib_cleanup.h"
+#include "celix_utils.h"
+#include "celix_version.h"
+#include "celix_framework_version.h"
+
+// Mandatory manifest attributes
+#define CELIX_BUNDLE_MANIFEST_VERSION "CELIX_BUNDLE_MANIFEST_VERSION"
+#define CELIX_BUNDLE_SYMBOLIC_NAME "CELIX_BUNDLE_SYMBOLIC_NAME"
+#define CELIX_BUNDLE_VERSION "CELIX_BUNDLE_VERSION"
+#define CELIX_BUNDLE_NAME "CELIX_BUNDLE_NAME"
+
+// Optional manifest attributes
+#define CELIX_BUNDLE_ACTIVATOR_LIBRARY "CELIX_BUNDLE_ACTIVATOR_LIBRARY"
+#define CELIX_BUNDLE_PRIVATE_LIBRARIES "CELIX_BUNDLE_PRIVATE_LIBRARIES"
+#define CELIX_BUNDLE_DESCRIPTION "CELIX_BUNDLE_DESCRIPTION"
+#define CELIX_BUNDLE_GROUP "CELIX_BUNDLE_GROUP"
+
+#define CELIX_BUNDLE_SYMBOLIC_NAME_ALLOWED_SPECIAL_CHARS "-_:."
+#define CELIX_FRAMEWORK_MANIFEST_VERSION "2.0.0"
+
+struct celix_bundle_manifest {
+    celix_properties_t* attributes;
+
+    //Mandatory fields
+    celix_version_t* manifestVersion;
+    celix_version_t* bundleVersion;
+    char* symbolicName;
+    char* bundleName;
+
+    //Optional fields
+    char* bundleGroup;
+    char* description;
+    char* activatorLibrary;
+    celix_array_list_t* privateLibraries;
+};
+
+/**
+ * @brief Set and validate the provided manifest by checking if all mandatory 
attributes are present and of the correct
+ * type and checking if the optional attributes, when present, are of the 
correct type.
+ */
+static celix_status_t 
celix_bundleManifest_setAttributes(celix_bundle_manifest_t* manifest);
+
+celix_status_t celix_bundleManifest_create(celix_properties_t* attributes, 
celix_bundle_manifest_t** manifestOut) {
+    if (!attributes) {
+        return CELIX_ILLEGAL_ARGUMENT;
+    }
+
+    celix_autoptr(celix_bundle_manifest_t) manifest = calloc(1, 
sizeof(*manifest));
+    if (!manifest) {
+        celix_properties_destroy(attributes);
+        celix_err_push("Failed to allocate memory for manifest");
+        return ENOMEM;
+    }
+    manifest->attributes = attributes;
+
+    celix_status_t status = celix_bundleManifest_setAttributes(manifest);
+    if (status != CELIX_SUCCESS) {
+        return status;
+    }
+
+    *manifestOut = celix_steal_ptr(manifest);
+    return CELIX_SUCCESS;
+}
+
+celix_status_t celix_bundleManifest_createFromFile(const char* filename, 
celix_bundle_manifest_t** manifestOut) {
+    celix_properties_t* properties = NULL;
+    celix_status_t status = celix_properties_load(filename, 0, &properties);
+    if (status != CELIX_SUCCESS) {
+        return status;
+    }
+    return celix_bundleManifest_create(properties, manifestOut);
+}
+
+celix_status_t 
celix_bundleManifest_createFrameworkManifest(celix_bundle_manifest_t** 
manifest) {
+    celix_autoptr(celix_properties_t) properties = celix_properties_create();
+    if (!properties) {
+        celix_err_push("Failed to create properties for framework manifest");
+        return ENOMEM;
+    }
+
+    celix_status_t status =
+        celix_properties_set(properties, CELIX_BUNDLE_MANIFEST_VERSION, 
CELIX_FRAMEWORK_MANIFEST_VERSION);
+    status = CELIX_DO_IF(status, celix_properties_set(properties, 
CELIX_BUNDLE_SYMBOLIC_NAME, "apache_celix_framework"));
+    status = CELIX_DO_IF(status, celix_properties_set(properties, 
CELIX_BUNDLE_NAME, "Apache Celix Framework"));
+    status = CELIX_DO_IF(status, celix_properties_set(properties, 
CELIX_BUNDLE_VERSION, CELIX_FRAMEWORK_VERSION));
+    status = CELIX_DO_IF(status, celix_properties_set(properties, 
CELIX_BUNDLE_GROUP, "Celix/Framework"));
+    status = CELIX_DO_IF(
+        status, celix_properties_set(properties, CELIX_BUNDLE_DESCRIPTION, 
"The Apache Celix Framework System Bundle"));
+
+    if (status != CELIX_SUCCESS) {
+        celix_err_push("Failed to set properties for framework manifest");
+        return status;
+    }
+
+    return celix_bundleManifest_create(celix_steal_ptr(properties), manifest);
+}
+
+void celix_bundleManifest_destroy(celix_bundle_manifest_t* manifest) {
+    if (manifest) {
+        celix_properties_destroy(manifest->attributes);
+
+        free(manifest->symbolicName);
+        free(manifest->bundleName);
+        celix_version_destroy(manifest->manifestVersion);
+        celix_version_destroy(manifest->bundleVersion);
+
+        free(manifest->activatorLibrary);
+        free(manifest->bundleGroup);
+        free(manifest->description);
+        celix_arrayList_destroy(manifest->privateLibraries);
+
+        free(manifest);
+    }
+}
+
+const celix_properties_t* 
celix_bundleManifest_getAttributes(celix_bundle_manifest_t* manifest) {
+    return manifest->attributes;
+}
+
+static celix_status_t 
celix_bundleManifest_setMandatoryAttributes(celix_bundle_manifest_t* manifest) {
+    const char* symbolicName = celix_properties_get(manifest->attributes, 
CELIX_BUNDLE_SYMBOLIC_NAME, NULL);
+    const char* bundleName = celix_properties_get(manifest->attributes, 
CELIX_BUNDLE_NAME, NULL);
+
+    celix_autoptr(celix_version_t) manifestVersion = NULL;
+    celix_status_t getVersionStatus =
+        celix_properties_getAsVersion(manifest->attributes, 
CELIX_BUNDLE_MANIFEST_VERSION, NULL, &manifestVersion);
+    CELIX_RETURN_IF_ENOMEM(getVersionStatus);
+
+    celix_autoptr(celix_version_t) bundleVersion = NULL;
+    getVersionStatus = celix_properties_getAsVersion(manifest->attributes, 
CELIX_BUNDLE_VERSION, NULL, &bundleVersion);
+    CELIX_RETURN_IF_ENOMEM(getVersionStatus);
+
+    celix_status_t status = CELIX_SUCCESS;
+    if (!bundleName) {
+        celix_err_push(CELIX_BUNDLE_NAME " is missing");
+        status = CELIX_ILLEGAL_ARGUMENT;
+    }
+    if (!symbolicName) {
+        celix_err_push(CELIX_BUNDLE_SYMBOLIC_NAME " is missing");
+        status = CELIX_ILLEGAL_ARGUMENT;
+    } else {
+        // check if bundle symbolic name only contains the following 
characters: [a-zA-Z0-9_-:]
+        for (size_t i = 0; symbolicName[i] != '\0'; ++i) {
+            if (!isalnum(symbolicName[i]) &&
+                strchr(CELIX_BUNDLE_SYMBOLIC_NAME_ALLOWED_SPECIAL_CHARS, 
symbolicName[i]) == NULL) {
+                celix_err_pushf(
+                    CELIX_BUNDLE_SYMBOLIC_NAME " '%s' contains invalid 
character '%c'", symbolicName, symbolicName[i]);
+                status = CELIX_ILLEGAL_ARGUMENT;
+                break;
+            }
+        }
+    }
+    if (!manifestVersion) {
+        celix_err_push(CELIX_BUNDLE_MANIFEST_VERSION " is missing or not a 
version");
+        status = CELIX_ILLEGAL_ARGUMENT;
+    }
+    if (!bundleVersion) {
+        celix_err_push(CELIX_BUNDLE_VERSION " is missing or not a version");
+        status = CELIX_ILLEGAL_ARGUMENT;
+    }
+
+    if (manifestVersion && celix_version_compareToMajorMinor(manifestVersion, 
2, 0) != 0) {
+        celix_err_push(CELIX_BUNDLE_MANIFEST_VERSION " is not 2.0.*");
+        status = CELIX_ILLEGAL_ARGUMENT;
+    }
+
+    if (status == CELIX_SUCCESS) {
+        manifest->symbolicName = celix_utils_strdup(symbolicName);
+        CELIX_RETURN_IF_NULL(manifest->symbolicName);
+        manifest->bundleName = celix_utils_strdup(bundleName);
+        CELIX_RETURN_IF_NULL(manifest->bundleName);
+        manifest->manifestVersion = celix_steal_ptr(manifestVersion);
+        manifest->bundleVersion = celix_steal_ptr(bundleVersion);
+    }
+
+    return status;
+}
+
+static celix_status_t 
celix_bundleManifest_setOptionalAttributes(celix_bundle_manifest_t* manifest) {
+    celix_status_t status = CELIX_SUCCESS;
+
+    const char* lib = celix_properties_getAsString(manifest->attributes, 
CELIX_BUNDLE_ACTIVATOR_LIBRARY, NULL);
+    celix_autofree char* activatorLib = NULL;
+    if (lib) {
+        activatorLib = celix_utils_strdup(lib);
+        CELIX_RETURN_IF_NULL(activatorLib);
+    }
+
+    const char* group = celix_properties_getAsString(manifest->attributes, 
CELIX_BUNDLE_GROUP, NULL);
+    celix_autofree char* bundleGroup = NULL;
+    if (group) {
+        bundleGroup = celix_utils_strdup(group);
+        CELIX_RETURN_IF_NULL(bundleGroup);
+    }
+
+    const char* desc = celix_properties_getAsString(manifest->attributes, 
CELIX_BUNDLE_DESCRIPTION, NULL);
+    celix_autofree char* description = NULL;
+    if (desc) {
+        description = celix_utils_strdup(desc);
+        CELIX_RETURN_IF_NULL(description);
+    }
+
+    celix_autoptr(celix_array_list_t) privateLibraries = NULL;
+    celix_status_t getStatus = celix_properties_getAsStringArrayList(
+        manifest->attributes, CELIX_BUNDLE_PRIVATE_LIBRARIES, NULL, 
&privateLibraries);
+    CELIX_RETURN_IF_ENOMEM(getStatus);
+    if (celix_properties_hasKey(manifest->attributes, 
CELIX_BUNDLE_PRIVATE_LIBRARIES) && !privateLibraries) {
+        celix_err_pushf(CELIX_BUNDLE_PRIVATE_LIBRARIES " is not a string 
array. Got: '%s'",
+                        celix_properties_get(manifest->attributes, 
CELIX_BUNDLE_PRIVATE_LIBRARIES, NULL));
+        status = CELIX_ILLEGAL_ARGUMENT;
+    }
+
+    if (status == CELIX_SUCCESS) {
+        manifest->activatorLibrary = celix_steal_ptr(activatorLib);
+        manifest->bundleGroup = celix_steal_ptr(bundleGroup);
+        manifest->description = celix_steal_ptr(description);
+        manifest->privateLibraries = celix_steal_ptr(privateLibraries);
+    }
+
+    return status;
+}
+
+static celix_status_t 
celix_bundleManifest_setAttributes(celix_bundle_manifest_t* manifest) {
+    celix_status_t mStatus = 
celix_bundleManifest_setMandatoryAttributes(manifest);
+    celix_status_t oStatus = 
celix_bundleManifest_setOptionalAttributes(manifest);
+    return mStatus != CELIX_SUCCESS ? mStatus : oStatus;
+}
+
+const char* celix_bundleManifest_getBundleName(celix_bundle_manifest_t* 
manifest) {
+    return manifest->bundleName;
+}
+
+const char* 
celix_bundleManifest_getBundleSymbolicName(celix_bundle_manifest_t* manifest) {
+    return manifest->symbolicName;
+}
+
+const celix_version_t* 
celix_bundleManifest_getBundleVersion(celix_bundle_manifest_t* manifest) {
+    return manifest->bundleVersion;

Review Comment:
   So this becomes 
   ```
   return celix_properties_getVersion(manifest->attributes, 
CELIX_BUNDLE_MANIFEST_VERSION);
   ```



##########
libs/framework/src/bundle.c:
##########
@@ -170,40 +156,23 @@ celix_status_t bundle_setState(bundle_pt bundle, 
bundle_state_e state) {
        return CELIX_SUCCESS;
 }
 
-celix_status_t bundle_createModule(bundle_pt bundle, module_pt* moduleOut) {
-       celix_status_t status = CELIX_SUCCESS;
-       bundle_archive_pt archive = NULL;
-       bundle_revision_pt revision = NULL;
-       manifest_pt headerMap = NULL;
-    long bundleId = 0;
-
-       status = CELIX_DO_IF(status, bundle_getArchive(bundle, &archive));
-       status = CELIX_DO_IF(status, bundleArchive_getCurrentRevision(archive, 
&revision));
-       status = CELIX_DO_IF(status, bundleRevision_getManifest(revision, 
&headerMap));
-    status = bundleArchive_getId(bundle->archive, &bundleId);
-
-    if (status != CELIX_SUCCESS) {
-        fw_logCode(bundle->framework->logger, CELIX_LOG_LEVEL_ERROR, status, 
"Cannot create module, cannot get bundle archive, revision, manifest or bundle 
id.");
-        return status;
-    }
+celix_status_t bundle_createModule(bundle_pt bundle, celix_module_t** 
moduleOut) {
+    celix_status_t status = CELIX_SUCCESS;
+    long bundleId = celix_bundle_getId(bundle);

Review Comment:
   Nice cleanup



##########
libs/framework/src/framework.c:
##########
@@ -2230,9 +2176,9 @@ static void 
celix_framework_printCelixErrForBundleEntry(celix_framework_t* frame
 celix_status_t celix_framework_startBundleEntry(celix_framework_t* framework, 
celix_bundle_entry_t* bndEntry) {
     assert(!celix_framework_isCurrentThreadTheEventLoop(framework));
     celix_status_t status = CELIX_SUCCESS;
-    const char* error = "";
+    const char* error = NULL;

Review Comment:
   Nice catch.



##########
libs/framework/src/celix_bundle_manifest.h:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.
+ */
+
+#ifndef CELIX_MANIFEST_H_
+#define CELIX_MANIFEST_H_
+
+#include "celix_array_list.h"
+#include "celix_cleanup.h"
+#include "celix_errno.h"
+#include "celix_properties_type.h"
+#include "celix_version_type.h"
+#include "celix_array_list_type.h"
+#include "celix_bundle_manifest_type.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @file celix_bundle_manifest.h
+ * @brief Header file for celix_bundle_manifest_t.
+ *
+ *
+ * The bundle manifest consists of a set of attributes, including a set of 
mandatory attributes.
+ * A bundle manifest is used to describe the contents of a bundle.
+ *
+ * A bundle manifest must contain the following attributes:
+ * - CELIX_BUNDLE_MANIFEST_VERSION, type celix_version_t, the version of the 
manifest.
+ * - CELIX_BUNDLE_SYMBOLIC_NAME, type string, the symbolic name of the bundle.
+ * - CELIX_BUNDLE_VERSION, type celix_version_t, the version of the bundle.
+ * - CELIX_BUNDLE_NAME, type string, the name of the bundle.
+ *
+ * The bundle manifest may contain the following attributes:
+ * - CELIX_BUNDLE_ACTIVATOR_LIBRARY, type string, the activator library of the 
bundle.
+ * - CELIX_BUNDLE_PRIVATE_LIBRARIES, type string array, the private libraries 
of the bundle.
+ * - CELIX_BUNDLE_GROUP, type string, the group of the bundle. Helps in 
grouping sets of bundles.
+ *
+ * And a manifest may contain any other attributes of any type, this can be 
retrieved using
+ * celix_bundleManifest_getAttributes.
+ *
+ * A bundle symbolic name can only contain the following characters: 
[a-zA-Z0-9_-:]
+ */
+
+/**
+ * Create a new manifest using the provided properties.
+ *
+ * If an error occurs, an error message is logged on celix_err and in case of 
an CELIX_ILLEGAL_ARGUMENT error, there
+ * can be multiple error messages.
+ *
+ * @param[in] properties The properties to use for the manifest. Takes 
ownership of the properties.
+ * @param[out] manifest The created manifest.
+ * @return CELIX_SUCCESS if no errors occurred, ENOMEM if memory allocation 
failed and CELIX_ILLEGAL_ARGUMENT if the
+ * provided attributes is incorrect. In case of an error, the provided 
attributes are destroyed.
+ */
+celix_status_t celix_bundleManifest_create(celix_properties_t* attributes, 
celix_bundle_manifest_t** manifest);
+
+/**
+ * Create a new manifest by reading the manifest from the provided file.
+ *
+ * If an error occurs, an error message is logged on celix_err.
+ *
+ * @param[in] filename The file to read the manifest from.
+ * @param[out] manifest The created manifest.
+ * @return CELIX_SUCCESS if no errors occurred, ENOMEM if memory allocation 
failed, CELIX_FILE_IO_EXCEPTION if the file
+ * could not be read and CELIX_ILLEGAL_ARGUMENT if the manifest file is 
invalid.
+ */
+celix_status_t celix_bundleManifest_createFromFile(const char* filename, 
celix_bundle_manifest_t** manifest);
+
+/**
+ * @brief Create a new framework manifest.
+ *
+ * The framework manifest is a special manifest that contains the following 
manifest attributes:
+ * - CELIX_BUNDLE_SYMBOLIC_NAME="celix.framework"
+ * - CELIX_BUNDLE_NAME="Celix Framework"
+ * - CELIX_BUNDLE_VERSION=CELIX_FRAMEWORK_VERSION
+ * - CELIX_BUNDLE_MANIFEST_VERSION="2.0.0"
+ * - CELIX_BUNDLE_GROUP="Celix"
+ * - CELIX_BUNDLE_DESCRIPTION="Celix Framework"
+ *
+ * @param manifest The created framework manifest.
+ * @return CELIX_SUCCESS if no errors occurred, ENOMEM if memory allocation 
failed. If an error occurs, an error message
+ * is logged on celix_err.
+ */
+celix_status_t 
celix_bundleManifest_createFrameworkManifest(celix_bundle_manifest_t** 
manifest);
+
+/**
+ * @brief Destroy the provided manifest.
+ */
+void celix_bundleManifest_destroy(celix_bundle_manifest_t* manifest);
+
+/**
+ * Define the cleanup function for a celix_bundleManifest_t, so that it can be 
used with celix_autoptr.
+ */
+CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_bundle_manifest_t, 
celix_bundleManifest_destroy)
+
+/**
+ * @brief Get the manifest attributes.
+ * The manifest attributes are valid as long as the manifest is valid.
+ *
+ * @param[in] manifest The bundle manifest to get the manifest version from. 
Cannot be NULL.
+ * @return The manifest attributes. Will never be NULL.

Review Comment:
   Nice to make the class-invariants explicit. I would suggest we do more this 
sort of things in the future.



##########
libs/framework/src/celix_bundle_manifest.c:
##########
@@ -0,0 +1,284 @@
+/*
+ * 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 "celix_bundle_manifest.h"
+#include <ctype.h>
+#include <string.h>
+
+#include "celix_err.h"
+#include "celix_properties.h"
+#include "celix_stdlib_cleanup.h"
+#include "celix_utils.h"
+#include "celix_version.h"
+#include "celix_framework_version.h"
+
+// Mandatory manifest attributes
+#define CELIX_BUNDLE_MANIFEST_VERSION "CELIX_BUNDLE_MANIFEST_VERSION"
+#define CELIX_BUNDLE_SYMBOLIC_NAME "CELIX_BUNDLE_SYMBOLIC_NAME"
+#define CELIX_BUNDLE_VERSION "CELIX_BUNDLE_VERSION"
+#define CELIX_BUNDLE_NAME "CELIX_BUNDLE_NAME"
+
+// Optional manifest attributes
+#define CELIX_BUNDLE_ACTIVATOR_LIBRARY "CELIX_BUNDLE_ACTIVATOR_LIBRARY"
+#define CELIX_BUNDLE_PRIVATE_LIBRARIES "CELIX_BUNDLE_PRIVATE_LIBRARIES"
+#define CELIX_BUNDLE_DESCRIPTION "CELIX_BUNDLE_DESCRIPTION"
+#define CELIX_BUNDLE_GROUP "CELIX_BUNDLE_GROUP"
+
+#define CELIX_BUNDLE_SYMBOLIC_NAME_ALLOWED_SPECIAL_CHARS "-_:."
+#define CELIX_FRAMEWORK_MANIFEST_VERSION "2.0.0"
+
+struct celix_bundle_manifest {
+    celix_properties_t* attributes;
+
+    //Mandatory fields
+    celix_version_t* manifestVersion;
+    celix_version_t* bundleVersion;
+    char* symbolicName;
+    char* bundleName;
+
+    //Optional fields
+    char* bundleGroup;
+    char* description;
+    char* activatorLibrary;
+    celix_array_list_t* privateLibraries;

Review Comment:
   How about enforcing strict typing of the attributes? So that instead of 
`celix_bundleManifest_setAttributes` we check all attributes whether mandatory 
or not are of the correct type. Then all the above fields will be gone. Not 
only is the implementation simpler (merely an encapsulated properties with 
extra type checking), it is more memory efficient.
   
   WDYT?



##########
libs/framework/src/bundle_archive.h:
##########
@@ -56,36 +56,36 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t 
bundleArchive_getId(bundle_arch
 CELIX_FRAMEWORK_EXPORT celix_status_t 
bundleArchive_getLocation(bundle_archive_pt archive, const char **location)
     __attribute__((deprecated("use celix_bundle_getLocation instead")));
 
-CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t 
bundleArchive_getArchiveRoot(bundle_archive_pt archive, const char 
**archiveRoot);
+CELIX_FRAMEWORK_DEPRECATED celix_status_t 
bundleArchive_getArchiveRoot(bundle_archive_pt archive, const char 
**archiveRoot);
 
-CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t
+CELIX_FRAMEWORK_DEPRECATED celix_status_t
 bundleArchive_revise(bundle_archive_pt archive, const char *location, const 
char *inputFile);
 
-CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t 
bundleArchive_rollbackRevise(bundle_archive_pt archive, bool *rolledback);
+CELIX_FRAMEWORK_DEPRECATED celix_status_t 
bundleArchive_rollbackRevise(bundle_archive_pt archive, bool *rolledback);
 
-CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t
-bundleArchive_getRevision(bundle_archive_pt archive, long revNr, 
bundle_revision_pt *revision);
+CELIX_FRAMEWORK_DEPRECATED celix_status_t
+bundleArchive_getRevision(bundle_archive_pt archive, long revNr, 
celix_bundle_revision_t** revision);
 
-CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t
-bundleArchive_getCurrentRevision(bundle_archive_pt archive, bundle_revision_pt 
*revision);
+CELIX_FRAMEWORK_DEPRECATED celix_status_t
+bundleArchive_getCurrentRevision(bundle_archive_pt archive, 
celix_bundle_revision_t** revision);
 
-CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t 
bundleArchive_getCurrentRevisionNumber(bundle_archive_pt archive, long 
*revisionNumber) __attribute__((deprecated));
+CELIX_FRAMEWORK_DEPRECATED celix_status_t 
bundleArchive_getCurrentRevisionNumber(bundle_archive_pt archive, long 
*revisionNumber) __attribute__((deprecated));

Review Comment:
   So the trailing attribute is unnecessary.



##########
libs/framework/src/celix_bundle_manifest.h:
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.
+ */
+
+#ifndef CELIX_MANIFEST_H_
+#define CELIX_MANIFEST_H_
+
+#include "celix_array_list.h"
+#include "celix_cleanup.h"
+#include "celix_errno.h"
+#include "celix_properties_type.h"
+#include "celix_version_type.h"
+#include "celix_array_list_type.h"
+#include "celix_bundle_manifest_type.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @file celix_bundle_manifest.h
+ * @brief Header file for celix_bundle_manifest_t.
+ *
+ *
+ * The bundle manifest consists of a set of attributes, including a set of 
mandatory attributes.
+ * A bundle manifest is used to describe the contents of a bundle.
+ *
+ * A bundle manifest must contain the following attributes:
+ * - CELIX_BUNDLE_MANIFEST_VERSION, type celix_version_t, the version of the 
manifest.
+ * - CELIX_BUNDLE_SYMBOLIC_NAME, type string, the symbolic name of the bundle.
+ * - CELIX_BUNDLE_VERSION, type celix_version_t, the version of the bundle.
+ * - CELIX_BUNDLE_NAME, type string, the name of the bundle.
+ *
+ * The bundle manifest may contain the following attributes:
+ * - CELIX_BUNDLE_ACTIVATOR_LIBRARY, type string, the activator library of the 
bundle.
+ * - CELIX_BUNDLE_PRIVATE_LIBRARIES, type string array, the private libraries 
of the bundle.
+ * - CELIX_BUNDLE_GROUP, type string, the group of the bundle. Helps in 
grouping sets of bundles.
+ *
+ * And a manifest may contain any other attributes of any type, this can be 
retrieved using
+ * celix_bundleManifest_getAttributes.
+ *
+ * A bundle symbolic name can only contain the following characters: 
[a-zA-Z0-9_-:]
+ */
+
+/**
+ * Create a new manifest using the provided properties.
+ *
+ * If an error occurs, an error message is logged on celix_err and in case of 
an CELIX_ILLEGAL_ARGUMENT error, there
+ * can be multiple error messages.
+ *
+ * @param[in] properties The properties to use for the manifest. Takes 
ownership of the properties.
+ * @param[out] manifest The created manifest.
+ * @return CELIX_SUCCESS if no errors occurred, ENOMEM if memory allocation 
failed and CELIX_ILLEGAL_ARGUMENT if the
+ * provided attributes is incorrect. In case of an error, the provided 
attributes are destroyed.
+ */
+celix_status_t celix_bundleManifest_create(celix_properties_t* attributes, 
celix_bundle_manifest_t** manifest);
+
+/**
+ * Create a new manifest by reading the manifest from the provided file.
+ *
+ * If an error occurs, an error message is logged on celix_err.
+ *
+ * @param[in] filename The file to read the manifest from.
+ * @param[out] manifest The created manifest.
+ * @return CELIX_SUCCESS if no errors occurred, ENOMEM if memory allocation 
failed, CELIX_FILE_IO_EXCEPTION if the file
+ * could not be read and CELIX_ILLEGAL_ARGUMENT if the manifest file is 
invalid.

Review Comment:
   Is `CELIX_INVALID_SYNTAX` more appropriate than `CELIX_ILLEGAL_ARGUMENT`? 
The argument is a filename in this case. If `filename == NULL`, then 
`CELIX_ILLEGAL_ARGUMENT` is returned. 
   
   Some subtleties lie here: `CELIX_ILLEGAL_ARGUMENT` implies caller's fault 
while `CELIX_INVALID_SYNTAX` implies manifest provider's fault.
   
   If the above holds, it also applies to `celix_bundleManifest_create`. 



##########
libs/framework/src/celix_bundle_private.h:
##########
@@ -67,6 +71,43 @@ bundle_archive_t *celix_bundle_getArchive(const 
celix_bundle_t *bundle);
  */
 celix_status_t bundle_destroy(celix_bundle_t *bundle);
 
+/**
+ * @brief Get the bundle symbolic name.

Review Comment:
   For `celix_bundle_getSymbolicName`?



##########
libs/framework/src/celix_module.h:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.
+ */
+
+#ifndef CELIX_MODULE_H_
+#define CELIX_MODULE_H_
+
+typedef struct celix_module celix_module_t;
+
+#include <stdbool.h>
+
+#include "celix_bundle_manifest.h"
+#include "celix_framework_export.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @brief Create a new module based on the provided manifest.
+ * @return A new module or NULL if the module could not be created. If NULL is 
returned, an error log is written to
+ * celix err.
+ */
+celix_module_t* module_create(celix_bundle_t* bundle);
+
+celix_module_t* module_createFrameworkModule(celix_framework_t* fw, 
celix_bundle_t *bundle);
+
+void module_destroy(celix_module_t* module);
+
+/**
+ * Define the cleanup function for a celix_bundleManifest_t, so that it can be 
used with celix_autoptr.
+ */
+CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_module_t, module_destroy)
+
+unsigned int module_hash(void *module);
+
+int module_equals(void *module, void *compare);

Review Comment:
   It seems that 
`module_hash`/`module_equals`/`module_getId`/`module_getDependents` are unused 
and undefined, and thus can be removed.



##########
cmake/cmake_celix/templates/MANIFEST.json.in:
##########
@@ -0,0 +1,14 @@
+{
+    $<JOIN:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_HEADERS>,$<COMMA>
+    
>$<$<BOOL:$<TARGET_PROPERTY:${BUNDLE_TARGET_NAME},BUNDLE_HEADERS>>:$<COMMA>>
+    "CELIX_BUNDLE_MANIFEST_VERSION" : "2.0.0",

Review Comment:
   Need to be a version string.



##########
libs/framework/src/module.c:
##########
@@ -44,183 +45,147 @@
 #define CELIX_LIBRARY_EXTENSION = ".dll";
 #endif
 
-struct module {
+struct celix_module {
     celix_framework_t* fw;
-
-    celix_version_t* version;
-    char* symbolicName;
-    char* group;
-    char* name;
-    char* description;

Review Comment:
   Nice cleanup.



##########
libs/framework/src/module.c:
##########
@@ -284,87 +256,53 @@ static celix_status_t 
celix_module_loadLibraryForManifestEntry(celix_module_t* m
     return status;
 }
 
-static celix_status_t 
celix_module_loadLibrariesInManifestEntry(celix_module_t* module, const char 
*librariesIn, const char *activator, bundle_archive_pt archive, void 
**activatorHandle) {
+static celix_status_t 
celix_module_loadLibrariesInManifestEntry(celix_module_t* module,
+                                                                const 
celix_array_list_t* libraries,
+                                                                const char* 
activator,
+                                                                
bundle_archive_pt archive,
+                                                                void** 
activatorHandle) {
+    assert(libraries != NULL);
+    assert(celix_arrayList_getElementType(libraries) == 
CELIX_ARRAY_LIST_ELEMENT_TYPE_STRING);
     celix_status_t status = CELIX_SUCCESS;
-    char* libraries = strndup(librariesIn, 1024*10);
 
-    char* saveptr1;
-    for (char* str1 = libraries; status == CELIX_SUCCESS; str1 = NULL) {
-        char* token = strtok_r(str1, ",", &saveptr1);
-        char* saveptr2;
-        if (token == NULL) {
+    for (int i = 0; i < celix_arrayList_size(libraries); i++) {
+        const char* library = celix_arrayList_getString(libraries, i);
+        void* handle = NULL;
+        status = celix_module_loadLibraryForManifestEntry(module, library, 
archive, &handle);
+        if (status != CELIX_SUCCESS) {
             break;
         }
-        char *pathToken = strtok_r(token, ";", &saveptr2);
-        if (pathToken == NULL) {
-            continue;
-        }
-
-        void *handle = NULL;
-        char lib[128];
-        lib[127] = '\0';
-        strncpy(lib, pathToken, 127);
-        char *trimmedLib = celix_utils_trimInPlace(lib);
-        status = celix_module_loadLibraryForManifestEntry(module, trimmedLib, 
archive, &handle);
-
-        if ( (status == CELIX_SUCCESS) && (activator != NULL) && 
(strcmp(trimmedLib, activator) == 0) ) {
+        if (activator != NULL && strcmp(library, activator) == 0) {
             *activatorHandle = handle;
         }
     }
-    free(libraries);
     return status;
 }
 
 celix_status_t celix_module_loadLibraries(celix_module_t* module) {
     celix_status_t status = CELIX_SUCCESS;
     celix_library_handle_t* activatorHandle = NULL;
-    bundle_archive_pt archive = NULL;
-    bundle_revision_pt revision = NULL;
-    manifest_pt manifest = NULL;
-
-    status = CELIX_DO_IF(status, bundle_getArchive(module->bundle, &archive));
-    status = CELIX_DO_IF(status, bundleArchive_getCurrentRevision(archive, 
&revision));
-    status = CELIX_DO_IF(status, bundleRevision_getManifest(revision, 
&manifest));
-    if (status == CELIX_SUCCESS) {
-        const char *privateLibraries = NULL;
-        const char *exportLibraries = NULL;
-        const char *activator = NULL;
-
-        privateLibraries = manifest_getValue(manifest, 
CELIX_FRAMEWORK_PRIVATE_LIBRARY);
-        exportLibraries = manifest_getValue(manifest, 
CELIX_FRAMEWORK_EXPORT_LIBRARY);
-        //@note not import yet
-        activator = manifest_getValue(manifest, 
CELIX_FRAMEWORK_BUNDLE_ACTIVATOR);
-
-        if (exportLibraries != NULL) {
-            status = CELIX_DO_IF(status, 
celix_module_loadLibrariesInManifestEntry(module, exportLibraries, activator, 
archive, &activatorHandle));

Review Comment:
   Now that we remove this and that we have already concluded export library 
does not work for C/C++, shall we remove it together with corresponding 
documentation and cmake commands?



##########
libs/utils/include/celix_err.h:
##########
@@ -94,6 +94,28 @@ CELIX_UTILS_EXPORT void celix_err_printErrors(FILE* stream, 
const char* prefix,
  */
 CELIX_UTILS_EXPORT int celix_err_dump(char* buf, size_t size, const char* 
prefix, const char* postfix);
 
+/*!

Review Comment:
   I just noticed that all usages of these macro are within 
celix_bundle_manifest.c. If we remove unnecessary duplication of manifest 
information, then all these usages are not needed any more.



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