This is an automated email from the ASF dual-hosted git repository.
pengzheng pushed a commit to branch feature/527-manifest-improvement
in repository https://gitbox.apache.org/repos/asf/celix.git
The following commit(s) were added to
refs/heads/feature/527-manifest-improvement by this push:
new 660994dd Fix broken manifest_clone.
660994dd is described below
commit 660994ddb7bfc0df9db617adadd0de03f54dc56c
Author: PengZheng <howto...@gmail.com>
AuthorDate: Fri Aug 11 21:33:45 2023 +0800
Fix broken manifest_clone.
Fix double free of keys when destroying the clone. Clone mainAttributes
when cloning.
---
libs/framework/gtest/src/ManifestTestSuite.cc | 80 ++++++++++++++++++----
libs/framework/include_deprecated/manifest.h | 4 ++
libs/framework/src/manifest.c | 31 +++++----
.../gtest/src/CelixUtilsAutoCleanupTestSuite.cc | 2 +-
4 files changed, 89 insertions(+), 28 deletions(-)
diff --git a/libs/framework/gtest/src/ManifestTestSuite.cc
b/libs/framework/gtest/src/ManifestTestSuite.cc
index ce58d294..401dfd04 100644
--- a/libs/framework/gtest/src/ManifestTestSuite.cc
+++ b/libs/framework/gtest/src/ManifestTestSuite.cc
@@ -26,22 +26,44 @@
class ManifestTestSuite : public ::testing::Test {
protected:
- manifest_pt manifest = nullptr;
- void SetUp() override {
- ASSERT_EQ(CELIX_SUCCESS, manifest_create(&manifest));
- }
- void TearDown() override {
- manifest_destroy(manifest);
- }
+ manifest_pt manifest = nullptr;
+ void SetUp() override {
+ ASSERT_EQ(CELIX_SUCCESS, manifest_create(&manifest));
+ }
+ void TearDown() override {
+ manifest_destroy(manifest);
+ }
+ void CheckPropertiesEqual(const celix_properties_t* prop1, const
celix_properties_t* prop2) {
+ EXPECT_EQ(celix_properties_size(prop1), celix_properties_size(prop2));
+ const char* key = nullptr;
+ CELIX_PROPERTIES_FOR_EACH(prop1, key) {
+ EXPECT_STREQ(celix_properties_get(prop1, key, nullptr),
celix_properties_get(prop2, key, nullptr));
+ }
+ }
+ void CheckManifestEqual(const manifest_pt manifest1, const manifest_pt
manifest2) {
+ CheckPropertiesEqual(manifest_getMainAttributes(manifest1),
manifest_getMainAttributes(manifest2));
+ hash_map_pt entries1 = nullptr;
+ EXPECT_EQ(CELIX_SUCCESS, manifest_getEntries(manifest1, &entries1));
+ hash_map_pt entries2 = nullptr;
+ EXPECT_EQ(CELIX_SUCCESS, manifest_getEntries(manifest2, &entries2));
+ EXPECT_EQ(hashMap_size(entries1), hashMap_size(entries2));
+ hash_map_iterator_t iter = hashMapIterator_construct(entries1);
+ while (hashMapIterator_hasNext(&iter)) {
+ hash_map_entry_pt entry = hashMapIterator_nextEntry(&iter);
+ celix_properties_t* value1 =
(celix_properties_t*)hashMapEntry_getValue(entry);
+ celix_properties_t* value2 =
(celix_properties_t*)hashMap_get(entries2, hashMapEntry_getKey(entry));
+ CheckPropertiesEqual(value1, value2);
+ }
+ }
};
TEST_F(ManifestTestSuite, EmptyManifestTest) {
- const celix_properties_t* mainAttr = manifest_getMainAttributes(manifest);
- EXPECT_EQ(0, celix_properties_size(mainAttr));
- hash_map_pt entries = nullptr;
- EXPECT_EQ(CELIX_SUCCESS, manifest_getEntries(manifest, &entries));
- EXPECT_NE(nullptr, entries);
- EXPECT_EQ(0, hashMap_size(entries));
+ const celix_properties_t* mainAttr = manifest_getMainAttributes(manifest);
+ EXPECT_EQ(0, celix_properties_size(mainAttr));
+ hash_map_pt entries = nullptr;
+ EXPECT_EQ(CELIX_SUCCESS, manifest_getEntries(manifest, &entries));
+ EXPECT_NE(nullptr, entries);
+ EXPECT_EQ(0, hashMap_size(entries));
}
TEST_F(ManifestTestSuite, ReadManifestWithoutNameSectionsTest) {
@@ -107,4 +129,36 @@ TEST_F(ManifestTestSuite,
ReadManifestWithNameSectionsTest) {
EXPECT_STREQ("N8Ow2UY4yjnHZv5zeq2I1Uv/+uE=", celix_properties_get(entry2,
"SHA1-Digest", nullptr));
EXPECT_STREQ("com.third._3d.native", celix_properties_get(entry2,
"Bundle-SymbolicName", nullptr));
EXPECT_STREQ("1.5.3", celix_properties_get(entry2, "Bundle-Version",
nullptr));
+}
+
+TEST_F(ManifestTestSuite, CleanupTest) {
+ celix_auto(manifest_pt) manifest2 = nullptr;
+ EXPECT_EQ(CELIX_SUCCESS, manifest_create(&manifest2));
+ celix_auto(manifest_pt) manifest3 = nullptr;
+ EXPECT_EQ(CELIX_SUCCESS, manifest_create(&manifest3));
+ EXPECT_EQ(CELIX_SUCCESS, manifest_destroy(celix_steal_ptr(manifest3)));
+}
+
+TEST_F(ManifestTestSuite, CloneTest) {
+ std::string content = "Manifest-Version: 1.0\n"
+ "DeploymentPackage-Icon: %icon\n"
+ "DeploymentPackage-SymbolicName: com.third._3d\n"
+ "DeploymentPacakge-Version: 1.2.3.build22032005\n"
+ "\n"
+ "Name: bundles/3dlib.jar\n"
+ "SHA1-Digest: MOez1l4gXHBo8ycYdAxstK3UvEg=\n"
+ "Bundle-SymbolicName: com.third._3d\n"
+ "Bundle-Version: 2.3.1\n"
+ "\n"
+ "Name: bundles/3dnative.jar\n"
+ "SHA1-Digest: N8Ow2UY4yjnHZv5zeq2I1Uv/+uE=\n"
+ "Bundle-SymbolicName: com.third._3d.native\n"
+ "Bundle-Version: 1.5.3\n"
+ "\n";
+ celix_autoptr(FILE) manifestFile = tmpfile();
+ EXPECT_EQ(content.size(), fwrite(content.c_str(), 1, content.size(),
manifestFile));
+ rewind(manifestFile);
+ EXPECT_EQ(CELIX_SUCCESS, manifest_readFromStream(manifest, manifestFile));
+ celix_auto(manifest_pt) clone = manifest_clone(manifest);
+ CheckManifestEqual(manifest, clone);
}
\ No newline at end of file
diff --git a/libs/framework/include_deprecated/manifest.h
b/libs/framework/include_deprecated/manifest.h
index 89e6f578..e4c4d629 100644
--- a/libs/framework/include_deprecated/manifest.h
+++ b/libs/framework/include_deprecated/manifest.h
@@ -27,9 +27,11 @@
#ifndef MANIFEST_H_
#define MANIFEST_H_
+#include <stddef.h>
#include <stdio.h>
#include "properties.h"
+#include "celix_cleanup.h"
#include "celix_errno.h"
#include "celix_framework_export.h"
@@ -52,6 +54,8 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t
manifest_createFromFile(const c
CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t manifest_destroy(manifest_pt
manifest);
+CELIX_DEFINE_AUTO_CLEANUP_FREE_FUNC(manifest_pt, manifest_destroy, NULL)
+
CELIX_FRAMEWORK_DEPRECATED_EXPORT void manifest_clear(manifest_pt manifest);
CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_properties_t
*manifest_getMainAttributes(manifest_pt manifest);
diff --git a/libs/framework/src/manifest.c b/libs/framework/src/manifest.c
index 11e83860..ca838f65 100644
--- a/libs/framework/src/manifest.c
+++ b/libs/framework/src/manifest.c
@@ -54,22 +54,25 @@ celix_status_t manifest_create(manifest_pt *manifest) {
}
manifest_pt manifest_clone(manifest_pt manifest) {
- celix_status_t status = CELIX_SUCCESS;
+ celix_status_t status = CELIX_SUCCESS;
- manifest_pt clone = NULL;
- status = manifest_create(&clone);
- if (status == CELIX_SUCCESS) {
- hash_map_iterator_t iter =
hashMapIterator_construct(manifest->attributes);
- while (hashMapIterator_hasNext(&iter)) {
- hash_map_entry_pt entry =
hashMapIterator_nextEntry(&iter);
- char *key = hashMapEntry_getKey(entry);
- celix_properties_t* value =
hashMapEntry_getValue(entry);
- celix_properties_t* cloneValue =
celix_properties_copy(value);
- hashMap_put(clone->attributes, key, cloneValue);
- }
- }
+ manifest_pt clone = NULL;
+ status = manifest_create(&clone);
+ if (status == CELIX_SUCCESS) {
+ const char* key = NULL;
+ CELIX_PROPERTIES_FOR_EACH(manifest->mainAttributes, key) {
+ celix_properties_set(clone->mainAttributes, key,
celix_properties_get(manifest->mainAttributes, key, NULL));
+ }
+ hash_map_iterator_t iter =
hashMapIterator_construct(manifest->attributes);
+ while (hashMapIterator_hasNext(&iter)) {
+ hash_map_entry_pt entry = hashMapIterator_nextEntry(&iter);
+ celix_properties_t* value = hashMapEntry_getValue(entry);
+ celix_properties_t* cloneValue = celix_properties_copy(value);
+ hashMap_put(clone->attributes, strdup(hashMapEntry_getKey(entry)),
cloneValue);
+ }
+ }
- return clone;
+ return clone;
}
celix_status_t manifest_destroy(manifest_pt manifest) {
diff --git a/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc
b/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc
index eeb3fae3..a74a6f33 100644
--- a/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc
+++ b/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc
@@ -192,4 +192,4 @@ TEST_F(CelixUtilsCleanupTestSuite, StealFdTest) {
TEST_F(CelixUtilsCleanupTestSuite, FileTest) {
celix_autoptr(FILE) file = tmpfile();
-}
\ No newline at end of file
+}
