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


##########
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:
   nicely stopped, I had not yet noticed that. Removed `createFrameworkModule`.



##########
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:
   nicely spotted, I had not yet noticed that. Removed `createFrameworkModule`.



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