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
commit 23a32b216a870039cb12ed18592fc1955cca15ee Author: PengZheng <[email protected]> AuthorDate: Fri Aug 11 20:10:27 2023 +0800 Fix memory leak in manifest_destroy. --- libs/framework/gtest/CMakeLists.txt | 1 + libs/framework/gtest/src/ManifestTestSuite.cc | 110 ++++++++++++ libs/framework/include_deprecated/manifest.h | 4 + libs/framework/src/manifest.c | 199 +++++++++++---------- .../gtest/src/CelixUtilsAutoCleanupTestSuite.cc | 5 + libs/utils/include/celix_stdio_cleanup.h | 36 ++++ 6 files changed, 264 insertions(+), 91 deletions(-) diff --git a/libs/framework/gtest/CMakeLists.txt b/libs/framework/gtest/CMakeLists.txt index 5ba22912..de6431f5 100644 --- a/libs/framework/gtest/CMakeLists.txt +++ b/libs/framework/gtest/CMakeLists.txt @@ -60,6 +60,7 @@ set(CELIX_FRAMEWORK_TEST_SOURCES src/CelixBundleCacheTestSuite.cc src/ScheduledEventTestSuite.cc src/FrameworkBundleTestSuite.cc + src/ManifestTestSuite.cc ) add_executable(test_framework ${CELIX_FRAMEWORK_TEST_SOURCES}) diff --git a/libs/framework/gtest/src/ManifestTestSuite.cc b/libs/framework/gtest/src/ManifestTestSuite.cc new file mode 100644 index 00000000..ce58d294 --- /dev/null +++ b/libs/framework/gtest/src/ManifestTestSuite.cc @@ -0,0 +1,110 @@ +/* + * 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 <gtest/gtest.h> +#include <string> + +#include "celix_stdio_cleanup.h" +#include "manifest.h" + +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); + } +}; + +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)); +} + +TEST_F(ManifestTestSuite, ReadManifestWithoutNameSectionsTest) { + std::string content = "Manifest-Version: 1.0\n" + "DeploymentPackage-SymbolicName: com.third._3d\n" + "DeploymentPacakge-Version: 1.2.3.build22032005\n" + "DeploymentPackage-Copyright: ACME Inc. (c) 2003\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)); + const celix_properties_t* mainAttr = manifest_getMainAttributes(manifest); + EXPECT_EQ(4, celix_properties_size(mainAttr)); + EXPECT_STREQ("1.0", celix_properties_get(mainAttr, "Manifest-Version", nullptr)); + EXPECT_STREQ("com.third._3d", celix_properties_get(mainAttr, "DeploymentPackage-SymbolicName", nullptr)); + EXPECT_STREQ("1.2.3.build22032005", celix_properties_get(mainAttr, "DeploymentPacakge-Version", nullptr)); + EXPECT_STREQ("ACME Inc. (c) 2003", celix_properties_get(mainAttr, "DeploymentPackage-Copyright", nullptr)); + 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, ReadManifestWithNameSectionsTest) { + 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)); + const celix_properties_t* mainAttr = manifest_getMainAttributes(manifest); + EXPECT_EQ(4, celix_properties_size(mainAttr)); + EXPECT_STREQ("1.0", celix_properties_get(mainAttr, "Manifest-Version", nullptr)); + EXPECT_STREQ("%icon", celix_properties_get(mainAttr, "DeploymentPackage-Icon", nullptr)); + EXPECT_STREQ("com.third._3d", celix_properties_get(mainAttr, "DeploymentPackage-SymbolicName", nullptr)); + EXPECT_STREQ("1.2.3.build22032005", celix_properties_get(mainAttr, "DeploymentPacakge-Version", nullptr)); + hash_map_pt entries = nullptr; + EXPECT_EQ(CELIX_SUCCESS, manifest_getEntries(manifest, &entries)); + EXPECT_NE(nullptr, entries); + EXPECT_EQ(2, hashMap_size(entries)); + + const celix_properties_t* entry1 = (const celix_properties_t*)hashMap_get(entries, "bundles/3dlib.jar"); + EXPECT_EQ(3, celix_properties_size(entry1)); + EXPECT_STREQ("MOez1l4gXHBo8ycYdAxstK3UvEg=", celix_properties_get(entry1, "SHA1-Digest", nullptr)); + EXPECT_STREQ("com.third._3d", celix_properties_get(entry1, "Bundle-SymbolicName", nullptr)); + EXPECT_STREQ("2.3.1", celix_properties_get(entry1, "Bundle-Version", nullptr)); + + const celix_properties_t* entry2 = (const celix_properties_t*)hashMap_get(entries, "bundles/3dnative.jar"); + EXPECT_EQ(3, celix_properties_size(entry2)); + 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)); +} \ No newline at end of file diff --git a/libs/framework/include_deprecated/manifest.h b/libs/framework/include_deprecated/manifest.h index cdd218cc..89e6f578 100644 --- a/libs/framework/include_deprecated/manifest.h +++ b/libs/framework/include_deprecated/manifest.h @@ -27,6 +27,8 @@ #ifndef MANIFEST_H_ #define MANIFEST_H_ +#include <stdio.h> + #include "properties.h" #include "celix_errno.h" #include "celix_framework_export.h" @@ -58,6 +60,8 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t manifest_getEntries(manifest_pt CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t manifest_read(manifest_pt manifest, const char *filename); +CELIX_FRAMEWORK_DEPRECATED_EXPORT celix_status_t manifest_readFromStream(manifest_pt manifest, FILE* stream); + CELIX_FRAMEWORK_DEPRECATED_EXPORT void manifest_write(manifest_pt manifest, const char *filename); CELIX_FRAMEWORK_DEPRECATED_EXPORT const char *manifest_getValue(manifest_pt manifest, const char *name); diff --git a/libs/framework/src/manifest.c b/libs/framework/src/manifest.c index 23db8fc5..11e83860 100644 --- a/libs/framework/src/manifest.c +++ b/libs/framework/src/manifest.c @@ -24,11 +24,11 @@ * \copyright Apache License, Version 2.0 */ -#include <stdio.h> #include <stdlib.h> #include <string.h> #include <stdbool.h> +#include "celix_stdio_cleanup.h" #include "manifest.h" #include "utils.h" #include "celix_log.h" @@ -73,15 +73,21 @@ manifest_pt manifest_clone(manifest_pt manifest) { } celix_status_t manifest_destroy(manifest_pt manifest) { - if (manifest != NULL) { - properties_destroy(manifest->mainAttributes); - hashMap_destroy(manifest->attributes, true, false); - manifest->mainAttributes = NULL; - manifest->attributes = NULL; - free(manifest); - manifest = NULL; - } - return CELIX_SUCCESS; + if (manifest != NULL) { + properties_destroy(manifest->mainAttributes); + 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_destroy(value); + } + hashMap_destroy(manifest->attributes, true, false); + manifest->mainAttributes = NULL; + manifest->attributes = NULL; + free(manifest); + manifest = NULL; + } + return CELIX_SUCCESS; } celix_status_t manifest_createFromFile(const char *filename, manifest_pt *manifest) { @@ -114,95 +120,106 @@ celix_status_t manifest_getEntries(manifest_pt manifest, hash_map_pt *map) { celix_status_t manifest_read(manifest_pt manifest, const char *filename) { celix_status_t status = CELIX_SUCCESS; - FILE *file = fopen ( filename, "r" ); - if (file != NULL) { - char lbuf[512]; - char name[512]; - bool skipEmptyLines = true; - char lastline[512]; - memset(lbuf,0,512); - memset(name,0,512); - memset(lastline,0,512); - - manifest_readAttributes(manifest, manifest->mainAttributes, file); - - while (status==CELIX_SUCCESS && fgets(lbuf, sizeof(lbuf), file) != NULL) { - properties_pt attributes; - int len = strlen(lbuf); - - if (lbuf[--len] != '\n') { - status = CELIX_FILE_IO_EXCEPTION; - framework_logIfError(celix_frameworkLogger_globalLogger(), status, NULL, "Manifest '%s' line too long", filename); - break; - } - if (len > 0 && lbuf[len - 1] == '\r') { - --len; - } - if (len == 0 && skipEmptyLines) { - continue; - } - skipEmptyLines = false; - - if (strlen(name) == 0) { - - if ((tolower(lbuf[0]) == 'n') && (tolower(lbuf[1]) == 'a') && - (tolower(lbuf[2]) == 'm') && (tolower(lbuf[3]) == 'e') && - (lbuf[4] == ':') && (lbuf[5] == ' ')) { - name[0] = '\0'; - strncpy(name, lbuf+6, len - 6); - name[len - 6] = '\0'; - } else { - status = CELIX_FILE_IO_EXCEPTION; - framework_logIfError(celix_frameworkLogger_globalLogger(), status, NULL, "Manifest '%s' invalid format", filename); - break; - } + celix_autoptr(FILE) file = fopen(filename, "r"); + if (file != NULL) { + status = manifest_readFromStream(manifest, file); + } else { + status = CELIX_FILE_IO_EXCEPTION; + } - if (fpeek(file) == ' ') { - int newlen = len - 6; - lastline[0] = '\0'; - strncpy(lastline, lbuf+6, len - 6); - lastline[newlen] = '\0'; - continue; - } - } else { - int newlen = strlen(lastline) + len; - char buf[512]; - buf[0] = '\0'; - strcpy(buf, lastline); - strncat(buf, lbuf+1, len - 1); - buf[newlen] = '\0'; - - if (fpeek(file) == ' ') { -// lastline = realloc(lastline, strlen(buf) + 1); - lastline[0] = '\0'; - strcpy(lastline, buf); - continue; - } - name[0] = '\0'; - strcpy(name, buf); - name[strlen(buf)] = '\0'; - } + if (status != CELIX_SUCCESS) { + fw_logCode(celix_frameworkLogger_globalLogger(), CELIX_LOG_LEVEL_ERROR, status, + "Cannot read manifest %s", filename); + } - attributes = hashMap_get(manifest->attributes, name); - if (attributes == NULL) { - attributes = properties_create(); - hashMap_put(manifest->attributes, strdup(name), attributes); - } - manifest_readAttributes(manifest, attributes, file); + return status; +} - name[0] = '\0'; - skipEmptyLines = true; - } - fclose(file); - } else { - status = CELIX_FILE_IO_EXCEPTION; - } +celix_status_t manifest_readFromStream(manifest_pt manifest, FILE* stream) { + celix_status_t status = CELIX_SUCCESS; + + char lbuf[512]; + char name[512]; + bool skipEmptyLines = true; + char lastline[512]; + memset(lbuf,0,512); + memset(name,0,512); + memset(lastline,0,512); + + manifest_readAttributes(manifest, manifest->mainAttributes, stream); + + while (status==CELIX_SUCCESS && fgets(lbuf, sizeof(lbuf), stream) != NULL) { + properties_pt attributes; + int len = strlen(lbuf); + + if (lbuf[--len] != '\n') { + status = CELIX_FILE_IO_EXCEPTION; + framework_logIfError(celix_frameworkLogger_globalLogger(), status, NULL, "Manifest line too long"); + break; + } + if (len > 0 && lbuf[len - 1] == '\r') { + --len; + } + if (len == 0 && skipEmptyLines) { + continue; + } + skipEmptyLines = false; + + if (strlen(name) == 0) { + + if ((tolower(lbuf[0]) == 'n') && (tolower(lbuf[1]) == 'a') && + (tolower(lbuf[2]) == 'm') && (tolower(lbuf[3]) == 'e') && + (lbuf[4] == ':') && (lbuf[5] == ' ')) { + name[0] = '\0'; + strncpy(name, lbuf+6, len - 6); + name[len - 6] = '\0'; + } else { + status = CELIX_FILE_IO_EXCEPTION; + framework_logIfError(celix_frameworkLogger_globalLogger(), status, NULL, "Manifest invalid format"); + break; + } + + if (fpeek(stream) == ' ') { + int newlen = len - 6; + lastline[0] = '\0'; + strncpy(lastline, lbuf+6, len - 6); + lastline[newlen] = '\0'; + continue; + } + } else { + int newlen = strlen(lastline) + len; + char buf[512]; + buf[0] = '\0'; + strcpy(buf, lastline); + strncat(buf, lbuf+1, len - 1); + buf[newlen] = '\0'; + + if (fpeek(stream) == ' ') { + lastline[0] = '\0'; + strcpy(lastline, buf); + continue; + } + name[0] = '\0'; + strcpy(name, buf); + name[strlen(buf)] = '\0'; + } + + attributes = hashMap_get(manifest->attributes, name); + if (attributes == NULL) { + attributes = properties_create(); + hashMap_put(manifest->attributes, strdup(name), attributes); + } + manifest_readAttributes(manifest, attributes, stream); + + name[0] = '\0'; + skipEmptyLines = true; + } if (status != CELIX_SUCCESS) { fw_logCode(celix_frameworkLogger_globalLogger(), CELIX_LOG_LEVEL_ERROR, status, "Cannot read manifest"); } - return status; + return status; } void manifest_write(manifest_pt manifest, const char * filename) { diff --git a/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc b/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc index 47fbc697..eeb3fae3 100644 --- a/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc +++ b/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include "celix_stdio_cleanup.h" #include "celix_stdlib_cleanup.h" #include "celix_threads.h" #include "celix_unistd_cleanup.h" @@ -187,4 +188,8 @@ TEST_F(CelixUtilsCleanupTestSuite, StealFdTest) { EXPECT_EQ(-1, fd); EXPECT_NE(-1, fd2); close(fd2); +} + +TEST_F(CelixUtilsCleanupTestSuite, FileTest) { + celix_autoptr(FILE) file = tmpfile(); } \ No newline at end of file diff --git a/libs/utils/include/celix_stdio_cleanup.h b/libs/utils/include/celix_stdio_cleanup.h new file mode 100644 index 00000000..43cbff16 --- /dev/null +++ b/libs/utils/include/celix_stdio_cleanup.h @@ -0,0 +1,36 @@ +/* + * 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_CELIX_STDIO_CLEANUP_H +#define CELIX_CELIX_STDIO_CLEANUP_H +#ifdef __cplusplus +extern "C" { +#endif + +#include <stdio.h> + +#include "celix_cleanup.h" + +CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(FILE, fclose) + +#ifdef __cplusplus +} +#endif +#endif // CELIX_CELIX_STDIO_CLEANUP_H
