This is an automated email from the ASF dual-hosted git repository. pengzheng pushed a commit to branch hotfix/convert_utils_fix in repository https://gitbox.apache.org/repos/asf/celix.git
commit 28b25c4afb576eecdef1c7ce984eaf11c67ccb6d Author: PengZheng <[email protected]> AuthorDate: Fri Apr 7 20:54:23 2023 +0800 Fix broken celix_utils_convertStringToDouble/celix_utils_convertStringToLong, and add more test cases. They don't work for strings consisting of whitespaces. --- libs/error_injector/malloc/src/malloc_ei.cc | 7 +++ libs/utils/CMakeLists.txt | 8 ++++ libs/utils/gtest/CMakeLists.txt | 1 + .../src/ConvertUtilsErrorInjectionTestSuite.cc | 46 ++++++++++++++++++ libs/utils/gtest/src/ConvertUtilsTestSuite.cc | 10 ++++ libs/utils/private/test/version_ei_test.cc | 56 ++++++++++++++++++++++ libs/utils/src/celix_convert_utils.c | 26 ++++------ libs/utils/src/version.c | 17 ++++--- 8 files changed, 148 insertions(+), 23 deletions(-) diff --git a/libs/error_injector/malloc/src/malloc_ei.cc b/libs/error_injector/malloc/src/malloc_ei.cc index 7d932e59..30997e33 100644 --- a/libs/error_injector/malloc/src/malloc_ei.cc +++ b/libs/error_injector/malloc/src/malloc_ei.cc @@ -18,26 +18,33 @@ */ #include "malloc_ei.h" +#include <errno.h> extern "C" { void *__real_malloc(size_t); CELIX_EI_DEFINE(malloc, void *) void *__wrap_malloc(size_t size) { + errno = ENOMEM; CELIX_EI_IMPL(malloc); + errno = 0; return __real_malloc(size); } void *__real_realloc(void *__ptr, size_t __size); CELIX_EI_DEFINE(realloc, void *) void *__wrap_realloc(void *__ptr, size_t __size) { + errno = ENOMEM; CELIX_EI_IMPL(realloc); + errno = 0; return __real_realloc(__ptr, __size); } void *__real_calloc (size_t __nmemb, size_t __size); CELIX_EI_DEFINE(calloc, void *) void *__wrap_calloc (size_t __nmemb, size_t __size) { + errno = ENOMEM; CELIX_EI_IMPL(calloc); + errno = 0; return __real_calloc(__nmemb, __size); } } \ No newline at end of file diff --git a/libs/utils/CMakeLists.txt b/libs/utils/CMakeLists.txt index f88f0f3e..ff1dd60f 100644 --- a/libs/utils/CMakeLists.txt +++ b/libs/utils/CMakeLists.txt @@ -134,6 +134,14 @@ if (ENABLE_TESTING) target_include_directories(version_test PRIVATE include_deprecated) target_link_libraries(version_test CppUTest::CppUTest Celix::utils pthread) + + if (LINKER_WRAP_SUPPORTED) + add_executable(version_ei_test private/test/version_ei_test.cc) + target_include_directories(version_ei_test PRIVATE include_deprecated) + target_link_libraries(version_ei_test CppUTest::CppUTest Celix::utils_obj Celix::malloc_ei Celix::utils_ei pthread) + add_test(NAME version_ei_test COMMAND version_ei_test) + endif () + configure_file(private/resources-test/properties.txt ${CMAKE_CURRENT_BINARY_DIR}/resources-test/properties.txt COPYONLY) add_test(NAME run_array_list_test COMMAND array_list_test) diff --git a/libs/utils/gtest/CMakeLists.txt b/libs/utils/gtest/CMakeLists.txt index 27482733..f4a3627a 100644 --- a/libs/utils/gtest/CMakeLists.txt +++ b/libs/utils/gtest/CMakeLists.txt @@ -79,6 +79,7 @@ setup_target_for_coverage(test_utils SCAN_DIR ..) if (LINKER_WRAP_SUPPORTED) add_executable(test_utils_with_ei src/FileUtilsErrorInjectionTestSuite.cc + src/ConvertUtilsErrorInjectionTestSuite.cc ) target_link_libraries(test_utils_with_ei PRIVATE Celix::zip_ei Celix::stdio_ei Celix::stat_ei Celix::fts_ei Celix::utils_obj Celix::utils_ei GTest::gtest GTest::gtest_main) target_include_directories(test_utils_with_ei PRIVATE ../src) #for version_private (needs refactoring of test) diff --git a/libs/utils/gtest/src/ConvertUtilsErrorInjectionTestSuite.cc b/libs/utils/gtest/src/ConvertUtilsErrorInjectionTestSuite.cc new file mode 100644 index 00000000..37d3c8a0 --- /dev/null +++ b/libs/utils/gtest/src/ConvertUtilsErrorInjectionTestSuite.cc @@ -0,0 +1,46 @@ +/* + 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_convert_utils.h" +#include "celix_utils_ei.h" +#include <gtest/gtest.h> + +class ConvertUtilsWithErrorInjectionTestSuite : public ::testing::Test { +public: + ~ConvertUtilsWithErrorInjectionTestSuite() override { + celix_ei_expect_celix_utils_writeOrCreateString(nullptr, 0, nullptr); + } +}; + +TEST_F(ConvertUtilsWithErrorInjectionTestSuite, CovertToBoolTest) { + bool converted; + celix_ei_expect_celix_utils_writeOrCreateString(CELIX_EI_UNKNOWN_CALLER, 0, nullptr); + bool result = celix_utils_convertStringToBool("true", false, &converted); + EXPECT_EQ(false, result); + EXPECT_FALSE(converted); +} + +TEST_F(ConvertUtilsWithErrorInjectionTestSuite, ConvertToVersionTest) { + celix_version_t* defaultVersion = celix_version_createVersion(1, 2, 3, "B"); + celix_ei_expect_celix_utils_writeOrCreateString(CELIX_EI_UNKNOWN_CALLER, 0, nullptr); + celix_version_t* result = celix_utils_convertStringToVersion("1.2.3", nullptr, nullptr); + EXPECT_EQ(nullptr, result); + + celix_version_destroy(defaultVersion); +} diff --git a/libs/utils/gtest/src/ConvertUtilsTestSuite.cc b/libs/utils/gtest/src/ConvertUtilsTestSuite.cc index 86ac327f..4ba4c209 100644 --- a/libs/utils/gtest/src/ConvertUtilsTestSuite.cc +++ b/libs/utils/gtest/src/ConvertUtilsTestSuite.cc @@ -51,6 +51,11 @@ TEST_F(ConvertUtilsTestSuite, ConvertToLongTest) { EXPECT_EQ(0, result); EXPECT_FALSE(converted); + //test for a string consisting of whitespaces + result = celix_utils_convertStringToLong(" ", 1, &converted); + EXPECT_EQ(1, result); + EXPECT_FALSE(converted); + //test for a string with a invalid number result = celix_utils_convertStringToLong("10A", 0, &converted); EXPECT_EQ(0, result); @@ -98,6 +103,11 @@ TEST_F(ConvertUtilsTestSuite, ConvertToDoubleTest) { EXPECT_EQ(0, result); EXPECT_FALSE(converted); + //test for an string consisting of whitespaces + result = celix_utils_convertStringToDouble(" ", 1.0, &converted); + EXPECT_EQ(1.0, result); + EXPECT_FALSE(converted); + //test for a string with a invalid number result = celix_utils_convertStringToDouble("10.5A", 0, &converted); EXPECT_FALSE(converted); diff --git a/libs/utils/private/test/version_ei_test.cc b/libs/utils/private/test/version_ei_test.cc new file mode 100644 index 00000000..66606ae4 --- /dev/null +++ b/libs/utils/private/test/version_ei_test.cc @@ -0,0 +1,56 @@ +/* + 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 <string.h> + +#include "celix_utils_ei.h" +#include "CppUTest/TestHarness.h" +#include "CppUTest/TestHarness_c.h" +#include "CppUTest/CommandLineTestRunner.h" +#include "malloc_ei.h" + +#include "celix_version.h" + +int main(int argc, char** argv) { + MemoryLeakWarningPlugin::turnOffNewDeleteOverloads(); + return RUN_ALL_TESTS(argc, argv); +} + +TEST_GROUP(version_ei) { + + void setup(void) { + celix_ei_expect_calloc(nullptr, 0, nullptr); + celix_ei_expect_celix_utils_strdup(nullptr, 0, nullptr); + } + + void teardown(void) { + celix_ei_expect_calloc(nullptr, 0, nullptr); + celix_ei_expect_celix_utils_strdup(nullptr, 0, nullptr); + } +}; + +TEST(version_ei, create) { + celix_ei_expect_calloc(CELIX_EI_UNKNOWN_CALLER, 0, nullptr); + celix_version_t *version = celix_version_createVersion(2, 2, 0, nullptr); + POINTERS_EQUAL(nullptr, version); + + celix_ei_expect_celix_utils_strdup(CELIX_EI_UNKNOWN_CALLER, 0, nullptr); + version = celix_version_createVersion(2, 2, 0, nullptr); + POINTERS_EQUAL(nullptr, version); +} diff --git a/libs/utils/src/celix_convert_utils.c b/libs/utils/src/celix_convert_utils.c index 22ec07d0..369db645 100644 --- a/libs/utils/src/celix_convert_utils.c +++ b/libs/utils/src/celix_convert_utils.c @@ -25,16 +25,7 @@ #include "celix_utils.h" #include "celix_version.h" -const char* celix_utils_eatWhitespace(const char* str) { - if (str != NULL) { - while (isspace(*str)) { - str++; - } - } - return str; -} - -bool celix_utils_isEndptrEndOfStringOrOnlyContainsWhitespaces(const char* endptr) { +static bool celix_utils_isEndptrEndOfStringOrOnlyContainsWhitespaces(const char* endptr) { bool result = false; if (endptr != NULL) { while (*endptr != '\0') { @@ -56,13 +47,16 @@ bool celix_utils_convertStringToBool(const char* val, bool defaultValue, bool* c if (val != NULL) { char buf[32]; char* valCopy = celix_utils_writeOrCreateString(buf, sizeof(buf), "%s", val); + if (valCopy == NULL) { + return result; + } char *trimmed = utils_stringTrim(valCopy); - if (strncasecmp("true", trimmed, 5) == 0) { + if (strcasecmp("true", trimmed) == 0) { result = true; if (converted) { *converted = true; } - } else if (strncasecmp("false", trimmed, 6) == 0) { + } else if (strcasecmp("false", trimmed) == 0) { result = false; if (converted) { *converted = true; @@ -80,7 +74,7 @@ double celix_utils_convertStringToDouble(const char* val, double defaultValue, b } if (val != NULL) { char *endptr; - double d = strtod(celix_utils_eatWhitespace(val), &endptr); + double d = strtod(val, &endptr); if (endptr != val && celix_utils_isEndptrEndOfStringOrOnlyContainsWhitespaces(endptr)) { result = d; if (converted) { @@ -98,7 +92,7 @@ long celix_utils_convertStringToLong(const char* val, long defaultValue, bool* c } if (val != NULL) { char *endptr; - long l = strtol(celix_utils_eatWhitespace(val), &endptr, 10); + long l = strtol(val, &endptr, 10); if (endptr != val && celix_utils_isEndptrEndOfStringOrOnlyContainsWhitespaces(endptr)) { result = l; if (converted) { @@ -118,8 +112,8 @@ celix_version_t* celix_utils_convertStringToVersion(const char* val, const celix if (firstDot != NULL && lastDot != NULL && firstDot != lastDot) { char buf[64]; char* valCopy = celix_utils_writeOrCreateString(buf, sizeof(buf), "%s", val); - char *trim = utils_stringTrim(valCopy); - result = celix_version_createVersionFromString(trim); + char *trimmed = utils_stringTrim(valCopy); + result = celix_version_createVersionFromString(trimmed); celix_utils_freeStringIfNotEqual(buf, valCopy); } } diff --git a/libs/utils/src/version.c b/libs/utils/src/version.c index f67bacc1..6c243ab5 100644 --- a/libs/utils/src/version.c +++ b/libs/utils/src/version.c @@ -122,13 +122,16 @@ celix_version_t* celix_version_createVersion(int major, int minor, int micro, co } celix_version_t* version = calloc(1, sizeof(*version)); - - version->major = major; - version->minor = minor; - version->micro = micro; - version->qualifier = celix_utils_strdup(qualifier); - - + if (version != NULL) { + version->major = major; + version->minor = minor; + version->micro = micro; + version->qualifier = celix_utils_strdup(qualifier); + if (version->qualifier == NULL) { + free(version); + version = NULL; + } + } return version; }
