This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/509-remove-cpputests in repository https://gitbox.apache.org/repos/asf/celix.git
commit 01b3af14dbf197239b7403e72cbb538e461a4099 Author: Pepijn Noltes <[email protected]> AuthorDate: Sat Dec 30 19:40:17 2023 +0100 Improve error handling of IP utils --- CHANGES.md | 3 ++ .../gtest/src/IpUtilsErrorInjectionTestSuite.cc | 58 ++++++++++++++++++---- libs/utils/gtest/src/IpUtilsTestSuite.cc | 12 +++-- libs/utils/include/celix_ip_utils.h | 15 ++++-- libs/utils/src/ip_utils.c | 30 ++++++----- 5 files changed, 88 insertions(+), 30 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1b712a70..fd5270b2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -53,6 +53,9 @@ limitations under the License. - C Properties are no longer a direct typedef of `hashmap`. - celix_string/long_hashmap put functions now return a celix_status_t instead of bool (value replaced). THe celix_status_t is used to indicate an ENOMEM error. +- linked_list.h is removed and no longer supported. Use celix_array_list.h instead. +- IP utils is refactored and the API is changed and all IP utils functions are now prefixed with `celix_utils_`. +- array_list.h is removed and no longer supported. Use celix_array_list.h instead. ## New Features diff --git a/libs/utils/gtest/src/IpUtilsErrorInjectionTestSuite.cc b/libs/utils/gtest/src/IpUtilsErrorInjectionTestSuite.cc index 93bd5626..8bbed74a 100644 --- a/libs/utils/gtest/src/IpUtilsErrorInjectionTestSuite.cc +++ b/libs/utils/gtest/src/IpUtilsErrorInjectionTestSuite.cc @@ -21,29 +21,67 @@ #include <errno.h> -#include "ifaddrs_ei.h" #include "celix_ip_utils.h" +#include "celix_err.h" + #include "celix_utils_ei.h" +#include "ifaddrs_ei.h" +#include "malloc_ei.h" class IpUtilsWithErrorInjectionTestSuite : public ::testing::Test { public: - IpUtilsWithErrorInjectionTestSuite() = default; + IpUtilsWithErrorInjectionTestSuite() { + celix_err_resetErrors(); + } + ~IpUtilsWithErrorInjectionTestSuite() override { celix_ei_expect_getifaddrs(nullptr, 0, 0); celix_ei_expect_celix_utils_strdup(nullptr, 0, nullptr); + celix_ei_expect_calloc(nullptr, 0, nullptr); + celix_err_resetErrors(); } }; -TEST_F(IpUtilsWithErrorInjectionTestSuite, failToGetInterfaceAddresses) { +TEST_F(IpUtilsWithErrorInjectionTestSuite, FailToGetInterfaceAddressesTest) { celix_ei_expect_getifaddrs((void *)&celix_utils_findIpInSubnet, 0, -1); - auto ipAddresses = celix_utils_findIpInSubnet("192.168.1.0/24"); - EXPECT_EQ(ipAddresses, nullptr); - EXPECT_EQ(errno, EMFILE); + char* ipAddr = nullptr; + auto status = celix_utils_findIpInSubnet("192.168.1.0/24", &ipAddr); + EXPECT_EQ(EMFILE, status); + EXPECT_EQ(ipAddr, nullptr); + EXPECT_EQ(1, celix_err_getErrorCount()); } -TEST_F(IpUtilsWithErrorInjectionTestSuite, failToDuplicateString) { +TEST_F(IpUtilsWithErrorInjectionTestSuite, FailToDuplicateStringTest) { + int errCount = 1; + char* ipAddr = nullptr; + + //first call to celix_utils_strdup fails celix_ei_expect_celix_utils_strdup((void *) &celix_utils_findIpInSubnet, 0, nullptr); - auto ipAddresses = celix_utils_findIpInSubnet("192.168.1.0/24"); - EXPECT_EQ(ipAddresses, nullptr); + auto status = celix_utils_findIpInSubnet("192.168.1.0/24", &ipAddr); + EXPECT_EQ(status, ENOMEM); + EXPECT_EQ(ipAddr, nullptr); + EXPECT_EQ(errCount++, celix_err_getErrorCount()); + + //second call to celix_utils_strdup fails (in ifa -> ifa_next loop) + celix_ei_expect_celix_utils_strdup((void *) &celix_utils_findIpInSubnet, 0, nullptr, 2); + status = celix_utils_findIpInSubnet("127.0.0.1/24", &ipAddr); + EXPECT_EQ(status, ENOMEM); + EXPECT_EQ(ipAddr, nullptr); + EXPECT_EQ(errCount++, celix_err_getErrorCount()); + + celix_ei_expect_celix_utils_strdup((void *) &celix_utils_convertIpToUint, 0, nullptr); + bool converted; + auto ipAsUint = celix_utils_convertIpToUint("192.168.1.0", &converted); + EXPECT_EQ(ipAsUint, 0); + EXPECT_FALSE(converted); + EXPECT_EQ(errno, ENOMEM); + EXPECT_EQ(errCount++, celix_err_getErrorCount()); +} + +TEST_F(IpUtilsWithErrorInjectionTestSuite, FailToCalledTest) { + celix_ei_expect_calloc((void*)celix_utils_convertUintToIp, 0, nullptr); + auto ip = celix_utils_convertUintToIp(3232235840); + EXPECT_EQ(ip, nullptr); EXPECT_EQ(errno, ENOMEM); -} \ No newline at end of file + EXPECT_EQ(1, celix_err_getErrorCount()); +} diff --git a/libs/utils/gtest/src/IpUtilsTestSuite.cc b/libs/utils/gtest/src/IpUtilsTestSuite.cc index 5a2d1aa4..27b5933e 100644 --- a/libs/utils/gtest/src/IpUtilsTestSuite.cc +++ b/libs/utils/gtest/src/IpUtilsTestSuite.cc @@ -108,18 +108,20 @@ TEST_F(IpUtilsTestSuite, NetmaskToPrefixTest) { } TEST_F(IpUtilsTestSuite, FindIpInSubnetWithInvalidInputTest) { - EXPECT_EQ(nullptr, celix_utils_findIpInSubnet("198.168.0.1")); // missing subnet + char* ipAddr = nullptr; + + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_utils_findIpInSubnet("198.168.0.1", &ipAddr)); // missing subnet EXPECT_EQ(1, celix_err_getErrorCount()); - EXPECT_EQ(nullptr, celix_utils_findIpInSubnet("198.168.0.1/abc")); // invalid subnet + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_utils_findIpInSubnet("198.168.0.1/abc", &ipAddr)); // invalid subnet EXPECT_EQ(2, celix_err_getErrorCount()); - EXPECT_EQ(nullptr, celix_utils_findIpInSubnet("198.168.0.1/40")); // out of range subnet + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_utils_findIpInSubnet("198.168.0.1/40", &ipAddr)); // out of range subnet EXPECT_EQ(3, celix_err_getErrorCount()); - EXPECT_EQ(nullptr, celix_utils_findIpInSubnet("198.168.0.1/-1")); // out of range subnet + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_utils_findIpInSubnet("198.168.0.1/-1", &ipAddr)); // out of range subnet EXPECT_EQ(4, celix_err_getErrorCount()); - EXPECT_EQ(nullptr, celix_utils_findIpInSubnet("a.b.c.d/8")); // invalid ip + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_utils_findIpInSubnet("a.b.c.d/8", &ipAddr)); // invalid ip EXPECT_EQ(5, celix_err_getErrorCount()); } diff --git a/libs/utils/include/celix_ip_utils.h b/libs/utils/include/celix_ip_utils.h index 3e487476..6ad89f07 100644 --- a/libs/utils/include/celix_ip_utils.h +++ b/libs/utils/include/celix_ip_utils.h @@ -28,10 +28,11 @@ #ifndef CELIX_IP_UTILS_H #define CELIX_IP_UTILS_H +#include "celix_errno.h" #include "celix_utils_export.h" -#include <stdint.h> #include <stdbool.h> +#include <stdint.h> #ifdef __cplusplus extern "C" { @@ -43,6 +44,8 @@ extern "C" { * This function takes an IP address as a string and converts it to * its corresponding unsigned integer representation. * + * If the conversation failed, an error message is logged to celix_err. + * * @param[in] ip The IP address in string format (e.g., "192.168.0.1"). * @param[out] converted A boolean indicating whether the conversion was successful. Can be NULL. * @return The IP address as an uint32_t. Returns 0 if the conversion fails. @@ -77,6 +80,8 @@ uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix); * This function converts a netmask in string format (e.g., "255.255.255.0") * to its corresponding prefix length. * + * If the conversation failed, an error message is logged to celix_err. + * * @param[in] netmask The netmask in string format. * @return The prefix length or -1 if the conversion fails. */ @@ -94,12 +99,16 @@ int celix_utils_ipNetmaskToPrefixLength(const char* netmask); * CIDR notation is a concise format for specifying IP addresses ranges using IP address and subnet prefix length. * It takes the form of 'IP_ADDRESS/PREFIX_LENGTH' (e.g., "192.168.0.1/24"). * + * If an error occurred, an error message is logged to celix_err. + * * @param[in] subnetCidrNotation The IP address with subnet prefix. - * @return A string containing an IP address within the specified subnet that is also + * @param[out] foundIp A string containing an IP address within the specified subnet that is also * assigned to a network interface on the host, or NULL if no matching IP address is found. The caller is owner of the * returned string. + * @return CELIX_SUCCESS if the IP address was found, CELIX_ILLEGAL_ARGUMENT if the subnet is invalid, CELIX_ENOMEM if + * an error occurred or a errno value set by getifaddrs. */ -char* celix_utils_findIpInSubnet(const char* subnetCidrNotation); +celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp); #ifdef __cplusplus } diff --git a/libs/utils/src/ip_utils.c b/libs/utils/src/ip_utils.c index 833bd203..31638201 100644 --- a/libs/utils/src/ip_utils.c +++ b/libs/utils/src/ip_utils.c @@ -25,6 +25,7 @@ #include "celix_convert_utils.h" #include <arpa/inet.h> +#include <assert.h> #include <errno.h> #include <ifaddrs.h> #include <limits.h> @@ -42,7 +43,7 @@ uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) { } // copy for strtok_r - celix_autofree char* input = strdup(ip); + celix_autofree char* input = celix_utils_strdup(ip); if (!input) { celix_err_push("Failed to duplicate input string for IP conversion"); return 0; @@ -125,14 +126,17 @@ int celix_utils_ipNetmaskToPrefixLength(const char* netmask) { return prefix; } -char* celix_utils_findIpInSubnet(const char* subnetCidrNotation) { - char* ip = NULL; +celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp) { + assert(subnetCidrNotation); + assert(foundIp); + + *foundIp = NULL; //create copy for strtok_r celix_autofree char* input = celix_utils_strdup(subnetCidrNotation); if (!input) { celix_err_push("Failed to duplicate input string for subnet search"); - return NULL; + return CELIX_ENOMEM; } char* savePtr; @@ -141,24 +145,24 @@ char* celix_utils_findIpInSubnet(const char* subnetCidrNotation) { if (!inputPrefixStr) { celix_err_pushf("Failed to parse IP address with prefix %s. Missing a '/'", subnetCidrNotation); - return NULL; + return CELIX_ILLEGAL_ARGUMENT; } bool convertedLong = false; int inputPrefix = (int)celix_utils_convertStringToLong(inputPrefixStr, INT_MAX, &convertedLong); if (!convertedLong) { celix_err_pushf("Failed to parse prefix in IP address with prefix %s", subnetCidrNotation); - return NULL; + return CELIX_ILLEGAL_ARGUMENT; } else if (inputPrefix > 32 || inputPrefix < 0) { celix_err_pushf( "Failed to parse IP address with prefix %s. Prefix %s is out of range", subnetCidrNotation, inputPrefixStr); - return NULL; + return CELIX_ILLEGAL_ARGUMENT; } bool converted; uint32_t ipAsUint = celix_utils_convertIpToUint(inputIp, &converted); if (!converted) { - return NULL; + return CELIX_ILLEGAL_ARGUMENT; } uint32_t bitmask = celix_utils_ipPrefixLengthToBitmask(inputPrefix); @@ -169,8 +173,9 @@ char* celix_utils_findIpInSubnet(const char* subnetCidrNotation) { struct ifaddrs *ifap, *ifa; if (getifaddrs(&ifap) == -1) { + celix_status_t status = errno; celix_err_push("Failed to get network interfaces"); - return NULL; + return status; } for (ifa = ifap; ifa; ifa = ifa->ifa_next) { @@ -202,13 +207,14 @@ char* celix_utils_findIpInSubnet(const char* subnetCidrNotation) { } if (ifIpAsUint >= ipRangeStart && ifIpAsUint <= ipRangeStop && inputPrefix >= ifPrefix) { - ip = celix_utils_strdup(if_addr); + char* ip = celix_utils_strdup(if_addr); if (!ip) { celix_err_push("Failed to duplicate IP address"); - break; + return CELIX_ENOMEM; } + *foundIp = ip; break; } } - return ip; + return CELIX_SUCCESS; }
