From 45bc7d2fffe28061ce0996ef80bb70ffc3f9b258 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Mon, 18 Dec 2023 20:26:56 +0800 Subject: [PATCH] Add more tests for dyn_common.c --- libs/dfi/gtest/CMakeLists.txt | 2 + libs/dfi/gtest/src/dyn_common_ei_tests.cc | 84 +++++++++++ libs/dfi/gtest/src/dyn_common_tests.cc | 149 +++++++++++++++++++ libs/dfi/src/dyn_common.c | 102 +++++++------ libs/dfi/src/dyn_common.h | 16 +- libs/error_injector/stdio/CMakeLists.txt | 1 + libs/error_injector/stdio/include/stdio_ei.h | 2 + libs/error_injector/stdio/src/stdio_ei.cc | 10 ++ 8 files changed, 299 insertions(+), 67 deletions(-) create mode 100644 libs/dfi/gtest/src/dyn_common_ei_tests.cc create mode 100644 libs/dfi/gtest/src/dyn_common_tests.cc diff --git a/libs/dfi/gtest/CMakeLists.txt b/libs/dfi/gtest/CMakeLists.txt index 5090bf3d5..9d3def82c 100644 --- a/libs/dfi/gtest/CMakeLists.txt +++ b/libs/dfi/gtest/CMakeLists.txt @@ -25,6 +25,7 @@ add_executable(test_dfi src/dyn_message_tests.cpp src/json_serializer_tests.cpp src/json_rpc_tests.cpp + src/dyn_common_tests.cc ) @@ -40,6 +41,7 @@ setup_target_for_coverage(test_dfi SCAN_DIR ..) if (EI_TESTS) add_executable(test_dfi_with_ei src/dyn_interface_ei_tests.cc + src/dyn_common_ei_tests.cc ) target_link_libraries(test_dfi_with_ei PRIVATE dfi_cut diff --git a/libs/dfi/gtest/src/dyn_common_ei_tests.cc b/libs/dfi/gtest/src/dyn_common_ei_tests.cc new file mode 100644 index 000000000..4efdf5d53 --- /dev/null +++ b/libs/dfi/gtest/src/dyn_common_ei_tests.cc @@ -0,0 +1,84 @@ +/* + * 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 "dyn_common.h" +#include "celix_err.h" +#include "malloc_ei.h" +#include "stdio_ei.h" + +#include + +class DynCommonErrorInjectionTestSuite : public ::testing::Test { +protected: + char* result{nullptr}; + FILE* stream{nullptr}; + + DynCommonErrorInjectionTestSuite() = default; + + ~DynCommonErrorInjectionTestSuite() override { + celix_ei_expect_calloc(nullptr, 0, nullptr); + celix_ei_expect_fclose(nullptr, 0, 0); + celix_ei_expect_fputc(nullptr, 0, 0); + celix_ei_expect_open_memstream(nullptr, 0, nullptr); + if (stream != nullptr) { + fclose(stream); + } + if (result != nullptr) { + free(result); + } + } + // delete other constructors and assign operators + DynCommonErrorInjectionTestSuite(DynCommonErrorInjectionTestSuite const&) = delete; + DynCommonErrorInjectionTestSuite(DynCommonErrorInjectionTestSuite&&) = delete; + DynCommonErrorInjectionTestSuite& operator=(DynCommonErrorInjectionTestSuite const&) = delete; + DynCommonErrorInjectionTestSuite& operator=(DynCommonErrorInjectionTestSuite&&) = delete; +}; + +TEST_F(DynCommonErrorInjectionTestSuite, ParseNameErrors) { + stream = fmemopen((void *) "valid_name", 10, "r"); + + // not enough memory for name + celix_ei_expect_open_memstream((void *) dynCommon_parseName, 1, nullptr); + ASSERT_EQ(dynCommon_parseName(stream, &result), 1); + std::string msg = "Error creating mem stream for name. "; + msg += strerror(ENOMEM); + ASSERT_STREQ(msg.c_str(), celix_err_popLastError()); + + // fail to put character into name + celix_ei_expect_fputc((void *) dynCommon_parseName, 1, EOF); + ASSERT_EQ(dynCommon_parseName(stream, &result), 1); + ASSERT_STREQ("Error writing to mem stream for name.", celix_err_popLastError()); + + // fail to close name stream + celix_ei_expect_fclose((void *) dynCommon_parseName, 1, EOF); + ASSERT_EQ(dynCommon_parseName(stream, &result), 1); + msg = "Error creating mem stream for name. "; + msg += strerror(ENOSPC); + ASSERT_STREQ(msg.c_str(), celix_err_popLastError()); +} + +TEST_F(DynCommonErrorInjectionTestSuite, ParseNameValueSectionErrors) { + stream = fmemopen((void*)"name1=value1\nname2=value2\n", 26, "r"); + struct namvals_head head; + TAILQ_INIT(&head); + // not enough memory for namval_entry when parsing name value section + celix_ei_expect_calloc((void *) dynCommon_parseNameValueSection, 0, nullptr, 1); + ASSERT_EQ(dynCommon_parseNameValueSection(stream, &head), 1); + ASSERT_STREQ("Error allocating memory for namval entry", celix_err_popLastError()); +} diff --git a/libs/dfi/gtest/src/dyn_common_tests.cc b/libs/dfi/gtest/src/dyn_common_tests.cc new file mode 100644 index 000000000..2848ab19a --- /dev/null +++ b/libs/dfi/gtest/src/dyn_common_tests.cc @@ -0,0 +1,149 @@ +/* + * 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 "dyn_common.h" +#include "celix_err.h" +#include +#include +#include + +class DynCommonTests : public ::testing::Test { +protected: + char* result{nullptr}; + FILE* stream{nullptr}; + DynCommonTests() { + } + ~DynCommonTests() override { + if (stream != nullptr) { + fclose(stream); + } + if (result != nullptr) { + free(result); + } + } + // delete other constructors and assign operators + DynCommonTests(DynCommonTests const&) = delete; + DynCommonTests(DynCommonTests&&) = delete; + DynCommonTests& operator=(DynCommonTests const&) = delete; + DynCommonTests& operator=(DynCommonTests&&) = delete; +}; + +TEST_F(DynCommonTests, ParseValidName) { + stream = fmemopen((void*)"valid_name", 10, "r"); + ASSERT_EQ(dynCommon_parseName(stream, &result), 0); + ASSERT_STREQ(result, "valid_name"); +} + +TEST_F(DynCommonTests, ParseNameTillInvalidCharacter) { + stream = fmemopen((void*)"invalid-name", 12, "r"); + ASSERT_EQ(dynCommon_parseName(stream, &result), 0); + ASSERT_STREQ("invalid", result); + ASSERT_EQ('-', fgetc(stream)); +} + +TEST_F(DynCommonTests, ParseNameWithExtraAllowableCharacters) { + stream = fmemopen((void*)"invalid-@name", 13, "r"); + ASSERT_EQ(dynCommon_parseNameAlsoAccept(stream, "@-", &result), 0); + ASSERT_STREQ("invalid-@name", result); + ASSERT_EQ(EOF, fgetc(stream)); + +} + +TEST_F(DynCommonTests, ParseEmptyName) { + stream = fmemopen((void*)"", 1, "r"); + ASSERT_EQ(dynCommon_parseName(stream, &result), 1); + ASSERT_EQ('\0', fgetc(stream)); +} + +TEST_F(DynCommonTests, EatCharValid) { + stream = fmemopen((void*)"valid", 5, "r"); + ASSERT_EQ(dynCommon_eatChar(stream, 'v'), 0); + ASSERT_EQ(dynCommon_eatChar(stream, 'a'), 0); + ASSERT_EQ(dynCommon_eatChar(stream, 'l'), 0); + ASSERT_EQ(dynCommon_eatChar(stream, 'i'), 0); + ASSERT_EQ(dynCommon_eatChar(stream, 'd'), 0); + ASSERT_FALSE(feof(stream)); + ASSERT_EQ(dynCommon_eatChar(stream, EOF), 0); + ASSERT_TRUE(feof(stream)); +} + +TEST_F(DynCommonTests, EatCharFromEmptyString) { + stream = fmemopen((void*)"", 1, "r"); + ASSERT_EQ(dynCommon_eatChar(stream, '\0'), 0); + ASSERT_FALSE(feof(stream)); + ASSERT_EQ(dynCommon_eatChar(stream, EOF), 0); + ASSERT_TRUE(feof(stream)); +} + +TEST_F(DynCommonTests, ParseNameValueSection) { + stream = fmemopen((void*)"name1=value1\nname2=value2\n", 26, "r"); + struct namvals_head head; + TAILQ_INIT(&head); + ASSERT_EQ(dynCommon_parseNameValueSection(stream, &head), 0); + + struct namval_entry *entry = TAILQ_FIRST(&head); + ASSERT_STREQ("name1", entry->name); + ASSERT_STREQ("value1", entry->value); + + entry = TAILQ_NEXT(entry, entries); + ASSERT_STREQ("name2", entry->name); + ASSERT_STREQ("value2", entry->value); + + dynCommon_clearNamValHead(&head); +} + +TEST_F(DynCommonTests, ParseNameValueSectionWithPairWithEmptyName) { + stream = fmemopen((void*)"=value1\nname2=value2\n", 22, "r"); + struct namvals_head head; + TAILQ_INIT(&head); + ASSERT_EQ(dynCommon_parseNameValueSection(stream, &head), 1); + ASSERT_STREQ("Parsed empty name", celix_err_popLastError()); + + dynCommon_clearNamValHead(&head); +} + +TEST_F(DynCommonTests, ParseNameValueSectionWithPairWithEmptyValue) { + stream = fmemopen((void*)"name1=\nname2=value2\n", 22, "r"); + struct namvals_head head; + TAILQ_INIT(&head); + ASSERT_EQ(dynCommon_parseNameValueSection(stream, &head), 1); + ASSERT_STREQ("Parsed empty name", celix_err_popLastError()); + + dynCommon_clearNamValHead(&head); +} + +TEST_F(DynCommonTests, ParseNameValueSectionWithPairMissingEquality) { + stream = fmemopen((void*)"name1 value1\nname2=value2\n", 22, "r"); + struct namvals_head head; + TAILQ_INIT(&head); + ASSERT_EQ(dynCommon_parseNameValueSection(stream, &head), 1); + ASSERT_STREQ("Error parsing, expected token '=' got ' ' at position 6", celix_err_popLastError()); + + dynCommon_clearNamValHead(&head); +} + +TEST_F(DynCommonTests, ParseNameValueSectionWithPairMissingNewline) { + stream = fmemopen((void*)"name1=value1 name2=value2\n", 26, "r"); + struct namvals_head head; + TAILQ_INIT(&head); + ASSERT_EQ(dynCommon_parseNameValueSection(stream, &head), 1); + ASSERT_STREQ("Error parsing, expected token '\n' got ' ' at position 13", celix_err_popLastError()); + + dynCommon_clearNamValHead(&head); +} \ No newline at end of file diff --git a/libs/dfi/src/dyn_common.c b/libs/dfi/src/dyn_common.c index 212e771ca..61029a5ce 100644 --- a/libs/dfi/src/dyn_common.c +++ b/libs/dfi/src/dyn_common.c @@ -19,6 +19,7 @@ #include "dyn_common.h" #include "celix_err.h" +#include "celix_stdio_cleanup.h" #include "celix_stdlib_cleanup.h" #include @@ -35,62 +36,59 @@ int dynCommon_parseName(FILE *stream, char **result) { } int dynCommon_parseNameAlsoAccept(FILE *stream, const char *acceptedChars, char **result) { - int status = OK; - - char *buf = NULL; + celix_autofree char *buf = NULL; size_t size = 0; - int strLen = 0; - FILE *name = open_memstream(&buf, &size); + celix_autoptr(FILE) name = open_memstream(&buf, &size); if (name == NULL) { celix_err_pushf("Error creating mem stream for name. %s", strerror(errno)); return ERROR; } int c = getc(stream); - while (isalnum(c) || c == '_' || (acceptedChars != NULL && strchr(acceptedChars, c) != NULL)) { - fputc(c, name); + while (c != EOF && (isalnum(c) || c == '_' || (acceptedChars != NULL && strchr(acceptedChars, c) != NULL))) { + if(fputc(c, name) == EOF) { + celix_err_push("Error writing to mem stream for name."); + return ERROR; + } c = getc(stream); - strLen += 1; } - fflush(name); - fclose(name); - ungetc(c, stream); - - if (status == OK) { - if (strLen == 0) { - status = ERROR; - celix_err_pushf("Parsed empty name"); - } + if (c != EOF) { + ungetc(c, stream); + } + if(fclose(celix_steal_ptr(name)) != 0) { + celix_err_pushf("Error creating mem stream for name. %s", strerror(errno)); + return ERROR; } - if (status == OK) { - *result = buf; - } else if (buf != NULL) { - free(buf); + if (size == 0) { + celix_err_pushf("Parsed empty name"); + return ERROR; } - return status; + *result = celix_steal_ptr(buf); + + return OK; } -int dynCommon_parseNameValue(FILE *stream, char **outName, char **outValue) { +static int dynCommon_parseNameValue(FILE *stream, char **outName, char **outValue) { int status; celix_autofree char *name = NULL; celix_autofree char *value = NULL; - - status = dynCommon_parseName(stream, &name); - if (status == OK) { - status = dynCommon_eatChar(stream, '='); - } - if (status == OK) { + do { + if ((status = dynCommon_parseName(stream, &name)) != OK) { + break; + } + if ((status = dynCommon_eatChar(stream, '=')) != OK) { + break; + } const char *valueAcceptedChars = ".<>{}[]?;:~!@#$%^&*()_+-=,./\\'\""; - - status = dynCommon_parseNameAlsoAccept(stream, valueAcceptedChars, &value); //NOTE use different more lenient function e.g. only stop at '\n' ? - } - - if (status == OK) { + //NOTE use different more lenient function e.g. only stop at '\n' ? + if ((status = dynCommon_parseNameAlsoAccept(stream, valueAcceptedChars, &value)) != OK) { + break; + } *outName = celix_steal_ptr(name); *outValue = celix_steal_ptr(value); - } + } while(false); return status; } @@ -129,31 +127,31 @@ int dynCommon_parseNameValueSection(FILE *stream, struct namvals_head *head) { celix_autofree char *name = NULL; celix_autofree char *value = NULL; - status = dynCommon_parseNameValue(stream, &name, &value); - - if (status == OK) { - status = dynCommon_eatChar(stream, '\n'); + if ((status = dynCommon_parseNameValue(stream, &name, &value)) != OK) { + break; } - struct namval_entry *entry = NULL; - if (status == OK) { - entry = calloc(1, sizeof(*entry)); - if (entry != NULL) { - entry->name = celix_steal_ptr(name); - entry->value = celix_steal_ptr(value); - TAILQ_INSERT_TAIL(head, entry, entries); - } else { - status = ERROR; - celix_err_pushf("Error allocating memory for namval entry"); - } + if ((status = dynCommon_eatChar(stream, '\n')) != OK) { + break; } - if (status != OK) { + struct namval_entry *entry = NULL; + entry = calloc(1, sizeof(*entry)); + if (entry == NULL) { + status = ERROR; + celix_err_pushf("Error allocating memory for namval entry"); break; } + + entry->name = celix_steal_ptr(name); + entry->value = celix_steal_ptr(value); + TAILQ_INSERT_TAIL(head, entry, entries); + peek = fgetc(stream); } - ungetc(peek, stream); + if (peek != EOF) { + ungetc(peek, stream); + } return status; } diff --git a/libs/dfi/src/dyn_common.h b/libs/dfi/src/dyn_common.h index 73e2fd933..ab74af5f9 100644 --- a/libs/dfi/src/dyn_common.h +++ b/libs/dfi/src/dyn_common.h @@ -40,7 +40,7 @@ struct namval_entry { /** * @brief Parse the name of dynamic type from the given stream. The name is only allowed to contain [a-zA-Z0-9_]. * - * The caller is the owner of the dynamic type name and use `free` deallocate the memory. + * The caller is the owner of the dynamic type name and use `free` to deallocate the memory. * * In case of an error, an error message is added to celix_err. * @@ -66,20 +66,6 @@ int dynCommon_parseName(FILE *stream, char **result); */ int dynCommon_parseNameAlsoAccept(FILE *stream, const char *acceptedChars, char **result); -/** - * @brief Parse the name and value of a name-value pair from the given stream. The name is only allowed to contain [a-zA-Z0-9_]. - * - * The caller is the owner of the name and value and use `free` deallocate the memory. - * - * In case of an error, an error message is added to celix_err. - * - * @param[in] stream The input stream. - * @param[out] name The name of the name-value pair. - * @param[out] value The value of the name-value pair. - * @return 0 if successful, otherwise 1. - */ -int dynCommon_parseNameValue(FILE *stream, char **name, char **value); - /** * @brief Parses a section of name-value pairs from the given stream. The name is only allowed to contain [a-zA-Z0-9_]. * diff --git a/libs/error_injector/stdio/CMakeLists.txt b/libs/error_injector/stdio/CMakeLists.txt index 8a4675b2e..f4824197e 100644 --- a/libs/error_injector/stdio/CMakeLists.txt +++ b/libs/error_injector/stdio/CMakeLists.txt @@ -30,5 +30,6 @@ target_link_options(stdio_ei INTERFACE LINKER:--wrap,fread LINKER:--wrap,fputc LINKER:--wrap,fputs + LINKER:--wrap,fclose ) add_library(Celix::stdio_ei ALIAS stdio_ei) diff --git a/libs/error_injector/stdio/include/stdio_ei.h b/libs/error_injector/stdio/include/stdio_ei.h index 210e546b9..321a39d9b 100644 --- a/libs/error_injector/stdio/include/stdio_ei.h +++ b/libs/error_injector/stdio/include/stdio_ei.h @@ -44,6 +44,8 @@ CELIX_EI_DECLARE(fputc, int); CELIX_EI_DECLARE(fputs, int); +CELIX_EI_DECLARE(fclose, int); + #ifdef __cplusplus } #endif diff --git a/libs/error_injector/stdio/src/stdio_ei.cc b/libs/error_injector/stdio/src/stdio_ei.cc index a3d3ee779..d78f7bbc2 100644 --- a/libs/error_injector/stdio/src/stdio_ei.cc +++ b/libs/error_injector/stdio/src/stdio_ei.cc @@ -98,4 +98,14 @@ int __wrap_fputs(const char* __s, FILE* __stream) { return __real_fputs(__s, __stream); } +int __real_fclose(FILE* __stream); +CELIX_EI_DEFINE(fclose, int) +int __wrap_fclose(FILE* __stream) { + int rc = __real_fclose(__stream); //note always call real fclose to ensure the stream is closed. + errno = ENOSPC; + CELIX_EI_IMPL(fclose); + errno = 0; + return rc; +} + }