You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by "pnoltes (via GitHub)" <gi...@apache.org> on 2024/01/01 18:56:48 UTC
[PR] Feature/509 remove cpputests (celix)
pnoltes opened a new pull request, #711:
URL: https://github.com/apache/celix/pull/711
# Intro
This PR removes the usage of Cpputest. Cpputest was used for the deprecated linked list, array_list tests, hash map, and IP utils.
# Linked List
This PR removes the linked list implementation and the linked list Cpputests. Linked list was still used in the shell TUI history, which has now been refactored to use the array list instead.
# Deprecated Array List
The header, implementation, and Cpputests for the deprecated array list API (API without the `celix_` prefix) have been removed. Additionally, any usage of the deprecated array list API has been updated to use celix_array_list. Note that the removal of the deprecated array list usage accounts for the majority of the changed files.
# IP Utils
The Cpputests for IP utils and their implementation have been refactored to GTests with an updated implementation. This includes enhanced error handling and corresponding tests. It's important to note that IP utils are currently not used in the Celix framework or bundles. However, they could be valuable for remote service implementations, specifically the `celix_utils_findIpInSubnet` function, which can identify the appropriate IP for listening based on a subnet CIDR notation.
# Deprecated Hash Map
The Cpputests for the deprecated hash map (hash map API without the `celix_` prefix) have been refactored to GTests and are maintained as much as possible in their original form.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/509 remove cpputests (celix)
Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on PR #711:
URL: https://github.com/apache/celix/pull/711#issuecomment-1876918450
> I also think this should be addressed in a separate ticket, if you agree I will make a ticket for this.
Yes, we can address this later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/509 remove cpputests (celix)
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on PR #711:
URL: https://github.com/apache/celix/pull/711#issuecomment-1876884143
> I saw lots of `arrayList` replaced by `celix_arrayList`, but only saw very few `#include "celix_array_list.h"`, which suggests that we depend heavily on "transtive includes". Transtive includes often increase accidental complexities. Removing an include from a public header file may break a lot of downstream builds, thus lead to unhappy users. I suggest we follow John Lakos' advice to forbid transtive includes "except to access a public base class of a type whose definition is included directly".
Also a good point. I do think this is more relevant for C++ code then for C code, but non the less also good to do for our C code.
If we forbid transtive dependencies, IMO we have to use forward declaration. So instead of including `celix_array_list.h` if a function returns or accepts a celix array list, we only need the following declaration:
`typedef struct celix_array_list celix_array_list_t;
I think it is then wise to define a guideline for this, something like:
For a C object, the following header guideline should be followed:
- The public api for a C object can can be found in a header named: `celix_<object_snake_name>.h`
- The opaque typedefs (forward declaration) can be found in a header named: `celix_<object_name_name>_type.h`. This header ideally should only contain opaque typedefs, but can also contain concrete struct definition if needed (e.g. `celix_properties_entry_t` or `celix_properties_value_type_e`, etc)
- If needed all private functions or structs (only used inside the library which provided the public api) can be found in a header named: `celix_<object_snake_name>_private.h`
- If needed all internal functions or structs (only used inside the Apache Celix project, but not meant as public API), can be found in a header named: ``celix_<object_snake_name>_internal.h`
I also think this should be addressed in a separate ticket, if you agree I will make a ticket for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/509 remove cpputests (celix)
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on code in PR #711:
URL: https://github.com/apache/celix/pull/711#discussion_r1441744141
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
if (ret != 1) {
+ celix_err_pushf("Failed to convert netmask %s to in_addr object", netmask);
return -1;
}
// Now convert the mask to a prefix
int prefix = 0;
- bool processed_one = false;
- unsigned int i = ntohl(in.s_addr);
+ uint32_t i = ntohl(in.s_addr);
while (i > 0) {
if (i & 1) {
prefix++;
- processed_one = true;
- } else {
- if (processed_one) return -1;
}
-
i >>= 1;
}
return prefix;
}
-/** Finds an IP of the available network interfaces of the machine by specifying an CIDR subnet.
- *
- * @param ipWithPrefix IP with prefix, e.g. 192.168.1.0/24
- * @return ip In case a matching interface could be found, an allocated string containing the IP of the
- * interface will be returned, e.g. 192.168.1.16. Memory for the new string can be freed with free().
- * When no matching interface is found NULL will be returned.
- */
-char *ipUtils_findIpBySubnet(const char *ipWithPrefix) {
- char *ip = NULL;
+celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp) {
+ assert(subnetCidrNotation);
+ assert(foundIp);
+
+ *foundIp = NULL;
- char *input = celix_utils_strdup(ipWithPrefix); // Make a copy as otherwise strtok_r manipulates the input string
- if (input == NULL) {
- goto strdup_failed;
+ //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 CELIX_ENOMEM;
}
- char *savePtr;
- char *inputIp = strtok_r(input, "/", &savePtr);
- char *inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int inputPrefix = (unsigned int) strtoul(inputPrefixStr, NULL, 10);
+ char* savePtr;
+ char* inputIp = strtok_r(input, "/", &savePtr);
+ char* inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int ipAsUint = ipUtils_ipToUnsignedInt(inputIp);
- unsigned int bitmask = ipUtils_prefixToBitmask(inputPrefix);
+ if (!inputPrefixStr) {
+ celix_err_pushf("Failed to parse IP address with prefix %s. Missing a '/'", subnetCidrNotation);
+ 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 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 CELIX_ILLEGAL_ARGUMENT;
+ }
+
+ bool converted;
+ uint32_t ipAsUint = celix_utils_convertIpToUint(inputIp, &converted);
+ if (!converted) {
+ return CELIX_ILLEGAL_ARGUMENT;
+ }
+ uint32_t bitmask = celix_utils_ipPrefixLengthToBitmask(inputPrefix);
unsigned int ipRangeStart = ipAsUint & bitmask;
unsigned int ipRangeStop = ipAsUint | ~bitmask;
// Requested IP range is known now, now loop through network interfaces
struct ifaddrs *ifap, *ifa;
- if(getifaddrs (&ifap) == -1) {
- goto getifaddrs_failed;
+ if (getifaddrs(&ifap) == -1) {
+ celix_status_t status = errno;
+ celix_err_push("Failed to get network interfaces");
+ return status;
}
+
for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
- if (ifa->ifa_addr == NULL)
+ if (ifa->ifa_addr == NULL) {
continue;
+ }
- if (ifa->ifa_addr->sa_family != AF_INET)
+ if (ifa->ifa_addr->sa_family != AF_INET) {
continue;
+ }
// Retrieve IP address for interface
char if_addr[NI_MAXHOST];
- int rv = getnameinfo(ifa->ifa_addr, sizeof(struct sockaddr_in),
- if_addr, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
+ int rv = getnameinfo(ifa->ifa_addr, sizeof(struct sockaddr_in), if_addr, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
Review Comment:
ip_utils.c is now removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/509 remove cpputests (celix)
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on code in PR #711:
URL: https://github.com/apache/celix/pull/711#discussion_r1441743916
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
if (ret != 1) {
+ celix_err_pushf("Failed to convert netmask %s to in_addr object", netmask);
return -1;
}
// Now convert the mask to a prefix
int prefix = 0;
- bool processed_one = false;
- unsigned int i = ntohl(in.s_addr);
+ uint32_t i = ntohl(in.s_addr);
while (i > 0) {
if (i & 1) {
prefix++;
- processed_one = true;
- } else {
- if (processed_one) return -1;
}
-
i >>= 1;
}
return prefix;
}
-/** Finds an IP of the available network interfaces of the machine by specifying an CIDR subnet.
- *
- * @param ipWithPrefix IP with prefix, e.g. 192.168.1.0/24
- * @return ip In case a matching interface could be found, an allocated string containing the IP of the
- * interface will be returned, e.g. 192.168.1.16. Memory for the new string can be freed with free().
- * When no matching interface is found NULL will be returned.
- */
-char *ipUtils_findIpBySubnet(const char *ipWithPrefix) {
- char *ip = NULL;
+celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp) {
+ assert(subnetCidrNotation);
+ assert(foundIp);
+
+ *foundIp = NULL;
- char *input = celix_utils_strdup(ipWithPrefix); // Make a copy as otherwise strtok_r manipulates the input string
- if (input == NULL) {
- goto strdup_failed;
+ //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 CELIX_ENOMEM;
}
- char *savePtr;
- char *inputIp = strtok_r(input, "/", &savePtr);
- char *inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int inputPrefix = (unsigned int) strtoul(inputPrefixStr, NULL, 10);
+ char* savePtr;
+ char* inputIp = strtok_r(input, "/", &savePtr);
+ char* inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int ipAsUint = ipUtils_ipToUnsignedInt(inputIp);
- unsigned int bitmask = ipUtils_prefixToBitmask(inputPrefix);
+ if (!inputPrefixStr) {
+ celix_err_pushf("Failed to parse IP address with prefix %s. Missing a '/'", subnetCidrNotation);
Review Comment:
ip_utils.c is now removed
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
if (ret != 1) {
+ celix_err_pushf("Failed to convert netmask %s to in_addr object", netmask);
return -1;
}
// Now convert the mask to a prefix
int prefix = 0;
- bool processed_one = false;
- unsigned int i = ntohl(in.s_addr);
+ uint32_t i = ntohl(in.s_addr);
while (i > 0) {
if (i & 1) {
prefix++;
- processed_one = true;
- } else {
- if (processed_one) return -1;
}
-
i >>= 1;
}
return prefix;
}
-/** Finds an IP of the available network interfaces of the machine by specifying an CIDR subnet.
- *
- * @param ipWithPrefix IP with prefix, e.g. 192.168.1.0/24
- * @return ip In case a matching interface could be found, an allocated string containing the IP of the
- * interface will be returned, e.g. 192.168.1.16. Memory for the new string can be freed with free().
- * When no matching interface is found NULL will be returned.
- */
-char *ipUtils_findIpBySubnet(const char *ipWithPrefix) {
- char *ip = NULL;
+celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp) {
+ assert(subnetCidrNotation);
+ assert(foundIp);
+
+ *foundIp = NULL;
- char *input = celix_utils_strdup(ipWithPrefix); // Make a copy as otherwise strtok_r manipulates the input string
- if (input == NULL) {
- goto strdup_failed;
+ //create copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(subnetCidrNotation);
Review Comment:
ip_utils.c is now removed
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
if (ret != 1) {
+ celix_err_pushf("Failed to convert netmask %s to in_addr object", netmask);
return -1;
}
// Now convert the mask to a prefix
int prefix = 0;
- bool processed_one = false;
- unsigned int i = ntohl(in.s_addr);
+ uint32_t i = ntohl(in.s_addr);
while (i > 0) {
if (i & 1) {
prefix++;
- processed_one = true;
- } else {
- if (processed_one) return -1;
}
-
i >>= 1;
}
return prefix;
}
-/** Finds an IP of the available network interfaces of the machine by specifying an CIDR subnet.
- *
- * @param ipWithPrefix IP with prefix, e.g. 192.168.1.0/24
- * @return ip In case a matching interface could be found, an allocated string containing the IP of the
- * interface will be returned, e.g. 192.168.1.16. Memory for the new string can be freed with free().
- * When no matching interface is found NULL will be returned.
- */
-char *ipUtils_findIpBySubnet(const char *ipWithPrefix) {
- char *ip = NULL;
+celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp) {
+ assert(subnetCidrNotation);
+ assert(foundIp);
+
+ *foundIp = NULL;
- char *input = celix_utils_strdup(ipWithPrefix); // Make a copy as otherwise strtok_r manipulates the input string
- if (input == NULL) {
- goto strdup_failed;
+ //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 CELIX_ENOMEM;
}
- char *savePtr;
- char *inputIp = strtok_r(input, "/", &savePtr);
- char *inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int inputPrefix = (unsigned int) strtoul(inputPrefixStr, NULL, 10);
+ char* savePtr;
+ char* inputIp = strtok_r(input, "/", &savePtr);
+ char* inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int ipAsUint = ipUtils_ipToUnsignedInt(inputIp);
- unsigned int bitmask = ipUtils_prefixToBitmask(inputPrefix);
+ if (!inputPrefixStr) {
+ celix_err_pushf("Failed to parse IP address with prefix %s. Missing a '/'", subnetCidrNotation);
+ 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 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 CELIX_ILLEGAL_ARGUMENT;
+ }
+
+ bool converted;
+ uint32_t ipAsUint = celix_utils_convertIpToUint(inputIp, &converted);
+ if (!converted) {
+ return CELIX_ILLEGAL_ARGUMENT;
+ }
+ uint32_t bitmask = celix_utils_ipPrefixLengthToBitmask(inputPrefix);
unsigned int ipRangeStart = ipAsUint & bitmask;
unsigned int ipRangeStop = ipAsUint | ~bitmask;
// Requested IP range is known now, now loop through network interfaces
struct ifaddrs *ifap, *ifa;
- if(getifaddrs (&ifap) == -1) {
- goto getifaddrs_failed;
+ if (getifaddrs(&ifap) == -1) {
+ celix_status_t status = errno;
+ celix_err_push("Failed to get network interfaces");
+ return status;
}
+
for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
- if (ifa->ifa_addr == NULL)
+ if (ifa->ifa_addr == NULL) {
continue;
+ }
- if (ifa->ifa_addr->sa_family != AF_INET)
+ if (ifa->ifa_addr->sa_family != AF_INET) {
continue;
+ }
// Retrieve IP address for interface
char if_addr[NI_MAXHOST];
- int rv = getnameinfo(ifa->ifa_addr, sizeof(struct sockaddr_in),
- if_addr, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
+ int rv = getnameinfo(ifa->ifa_addr, sizeof(struct sockaddr_in), if_addr, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
if (rv != 0) {
- printf("getnameinfo() failed: %s\n", gai_strerror(rv));
+ celix_err_pushf("Failed to get IP address for interface %s: %s", ifa->ifa_name, gai_strerror(rv));
continue;
}
// Retrieve netmask
- struct sockaddr_in *sa = (struct sockaddr_in *) ifa->ifa_netmask;
- char *if_netmask = inet_ntoa(sa->sin_addr);
+ struct sockaddr_in* sa = (struct sockaddr_in*)ifa->ifa_netmask;
+ char* if_netmask = inet_ntoa(sa->sin_addr);
Review Comment:
ip_utils.c is now removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/509 remove cpputests (celix)
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on code in PR #711:
URL: https://github.com/apache/celix/pull/711#discussion_r1441745775
##########
libs/utils/src/hash_map_private.h:
##########
@@ -29,13 +29,17 @@
#include "hash_map.h"
-unsigned int hashMap_hashCode(const void* toHash);
-int hashMap_equals(const void* toCompare, const void* compare);
+#ifdef __cplusplus
+extern "C" {
+#endif
-void hashMap_resize(hash_map_pt map, int newCapacity);
-hash_map_entry_pt hashMap_removeEntryForKey(hash_map_pt map, const void* key);
-hash_map_entry_pt hashMap_removeMapping(hash_map_pt map, hash_map_entry_pt entry);
-void hashMap_addEntry(hash_map_pt map, int hash, void* key, void* value, int bucketIndex);
+CELIX_UTILS_DEPRECATED_EXPORT unsigned int hashMap_hashCode(const void* toHash);
Review Comment:
The export macros are indeed not needed, removing them. Deprecated is correct but IMO mainly valuable on a public api.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/509 remove cpputests (celix)
Posted by "PengZheng (via GitHub)" <gi...@apache.org>.
PengZheng commented on code in PR #711:
URL: https://github.com/apache/celix/pull/711#discussion_r1439133798
##########
bundles/remote_services/discovery_common/include/endpoint_discovery_poller.h:
##########
@@ -55,6 +55,6 @@ celix_status_t endpointDiscoveryPoller_destroy(endpoint_discovery_poller_t *poll
celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url);
celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url);
-celix_status_t endpointDiscoveryPoller_getDiscoveryEndpoints(endpoint_discovery_poller_t *poller, array_list_pt urls);
+celix_status_t endpointDiscoveryPoller_getDiscoveryEndpoints(endpoint_discovery_poller_t *poller, celix_array_list_t* urls);
Review Comment:
Missing `#include "celix_array_list.h"`.
##########
bundles/remote_services/discovery_common/src/endpoint_discovery_poller.c:
##########
@@ -183,111 +184,118 @@ celix_status_t endpointDiscoveryPoller_getDiscoveryEndpoints(endpoint_discovery_
/**
* Adds a new endpoint URL to the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status;
+celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status;
- status = celixThreadMutex_lock(&(poller)->pollerLock);
- if (status != CELIX_SUCCESS) {
- return CELIX_BUNDLE_EXCEPTION;
- }
+ status = celixThreadMutex_lock(&(poller)->pollerLock);
+ if (status != CELIX_SUCCESS) {
+ return CELIX_BUNDLE_EXCEPTION;
+ }
- // Avoid memory leaks when adding an already existing URL...
- array_list_pt endpoints = hashMap_get(poller->entries, url);
- if (endpoints == NULL) {
- status = arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &endpoints);
+ // Avoid memory leaks when adding an already existing URL...
+ celix_array_list_t* endpoints = hashMap_get(poller->entries, url);
+ if (endpoints == NULL) {
+ endpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
- if (status == CELIX_SUCCESS) {
+ if (endpoints) {
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: add new discovery endpoint with url %s", url);
- hashMap_put(poller->entries, strdup(url), endpoints);
- endpointDiscoveryPoller_poll(poller, url, endpoints);
- }
- }
+ hashMap_put(poller->entries, strdup(url), endpoints);
+ endpointDiscoveryPoller_poll(poller, url, endpoints);
+ }
+ }
- status = celixThreadMutex_unlock(&poller->pollerLock);
+ status = celixThreadMutex_unlock(&poller->pollerLock);
- return status;
+ return status;
}
/**
* Removes an endpoint URL from the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status = CELIX_SUCCESS;
-
- if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
- status = CELIX_BUNDLE_EXCEPTION;
- } else {
- hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
-
- if (entry == NULL) {
- celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?", url);
- } else {
- char* origKey = hashMapEntry_getKey(entry);
+celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status = CELIX_SUCCESS;
+
+ if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
+ status = CELIX_BUNDLE_EXCEPTION;
+ } else {
+ hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
+
+ if (entry == NULL) {
+ celix_logHelper_debug(
+ *poller->loghelper,
+ "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?",
+ url);
+ } else {
+ char* origKey = hashMapEntry_getKey(entry);
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: remove discovery endpoint with url %s", url);
- array_list_pt entries = hashMap_remove(poller->entries, url);
+ celix_array_list_t* entries = hashMap_remove(poller->entries, url);
- if (entries != NULL) {
- for (unsigned int i = arrayList_size(entries); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(entries, i - 1);
- discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(entries, i - 1);
- endpointDescription_destroy(endpoint);
- }
- arrayList_destroy(entries);
- }
+ if (entries != NULL) {
+ for (unsigned int i = celix_arrayList_size(entries); i > 0; i--) {
+ endpoint_description_t* endpoint = celix_arrayList_get(entries, i - 1);
+ discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
+ celix_arrayList_removeAt(entries, i - 1);
+ endpointDescription_destroy(endpoint);
+ }
+ celix_arrayList_destroy(entries);
+ }
- free(origKey);
- }
- status = celixThreadMutex_unlock(&poller->pollerLock);
- }
+ free(origKey);
+ }
+ status = celixThreadMutex_unlock(&poller->pollerLock);
+ }
- return status;
+ return status;
}
-
-
-
-celix_status_t endpointDiscoveryPoller_poll(endpoint_discovery_poller_t *poller, char *url, array_list_pt currentEndpoints) {
- celix_status_t status;
- array_list_pt updatedEndpoints = NULL;
-
- // create an arraylist with a custom equality test to ensure we can find endpoints properly...
- arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &updatedEndpoints);
- status = endpointDiscoveryPoller_getEndpoints(poller, url, &updatedEndpoints);
-
- if (status == CELIX_SUCCESS) {
- if (updatedEndpoints != NULL) {
- for (unsigned int i = arrayList_size(currentEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(currentEndpoints, i - 1);
-
- if (!arrayList_contains(updatedEndpoints, endpoint)) {
- status = discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(currentEndpoints, i - 1);
- endpointDescription_destroy(endpoint);
- }
- }
-
- for (int i = arrayList_size(updatedEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_remove(updatedEndpoints, 0);
-
- if (!arrayList_contains(currentEndpoints, endpoint)) {
- arrayList_add(currentEndpoints, endpoint);
- status = discovery_addDiscoveredEndpoint(poller->discovery, endpoint);
- } else {
- endpointDescription_destroy(endpoint);
-
- }
- }
- }
- }
-
- if (updatedEndpoints != NULL) {
- arrayList_destroy(updatedEndpoints);
- }
-
- return status;
+celix_status_t
+endpointDiscoveryPoller_poll(endpoint_discovery_poller_t* poller, char* url, celix_array_list_t* currentEndpoints) {
+ celix_status_t status;
+ celix_array_list_t* updatedEndpoints = NULL;
+
+ // create an arraylist with a custom equality test to ensure we can find endpoints properly...
+ updatedEndpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
Review Comment:
If `updatedEndpoints == NULL`, we need to return error early. Otherwise, `endpointDiscoveryPoller_getEndpoints` will call normal `celix_arrayList_create` to try to create it again, which is inconsistent with `celix_arrayList_createWithEquals`.
##########
bundles/remote_services/topology_manager/src/scope.c:
##########
@@ -150,12 +153,15 @@ celix_status_t tm_removeImportScope(void *handle, char *filter) {
}
if (celixThreadMutex_lock(&scope->importScopeLock) == CELIX_SUCCESS) {
- int index = arrayList_indexOf(scope->importScopes, new);
- filter_pt present = (filter_pt) arrayList_get(scope->importScopes, index);
+ celix_array_list_entry_t entry;
+ memset(&entry, 0, sizeof(entry));
+ entry.voidPtrVal = new;
+ int index = celix_arrayList_indexOf(scope->importScopes, entry);
Review Comment:
The above remark also applies here.
##########
bundles/remote_services/discovery_common/src/endpoint_discovery_poller.c:
##########
@@ -183,111 +184,118 @@ celix_status_t endpointDiscoveryPoller_getDiscoveryEndpoints(endpoint_discovery_
/**
* Adds a new endpoint URL to the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status;
+celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status;
- status = celixThreadMutex_lock(&(poller)->pollerLock);
- if (status != CELIX_SUCCESS) {
- return CELIX_BUNDLE_EXCEPTION;
- }
+ status = celixThreadMutex_lock(&(poller)->pollerLock);
+ if (status != CELIX_SUCCESS) {
+ return CELIX_BUNDLE_EXCEPTION;
+ }
- // Avoid memory leaks when adding an already existing URL...
- array_list_pt endpoints = hashMap_get(poller->entries, url);
- if (endpoints == NULL) {
- status = arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &endpoints);
+ // Avoid memory leaks when adding an already existing URL...
+ celix_array_list_t* endpoints = hashMap_get(poller->entries, url);
+ if (endpoints == NULL) {
+ endpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
- if (status == CELIX_SUCCESS) {
+ if (endpoints) {
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: add new discovery endpoint with url %s", url);
- hashMap_put(poller->entries, strdup(url), endpoints);
- endpointDiscoveryPoller_poll(poller, url, endpoints);
- }
- }
+ hashMap_put(poller->entries, strdup(url), endpoints);
+ endpointDiscoveryPoller_poll(poller, url, endpoints);
+ }
+ }
- status = celixThreadMutex_unlock(&poller->pollerLock);
+ status = celixThreadMutex_unlock(&poller->pollerLock);
- return status;
+ return status;
}
/**
* Removes an endpoint URL from the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status = CELIX_SUCCESS;
-
- if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
- status = CELIX_BUNDLE_EXCEPTION;
- } else {
- hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
-
- if (entry == NULL) {
- celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?", url);
- } else {
- char* origKey = hashMapEntry_getKey(entry);
+celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status = CELIX_SUCCESS;
+
+ if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
+ status = CELIX_BUNDLE_EXCEPTION;
+ } else {
+ hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
+
+ if (entry == NULL) {
+ celix_logHelper_debug(
+ *poller->loghelper,
+ "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?",
+ url);
+ } else {
+ char* origKey = hashMapEntry_getKey(entry);
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: remove discovery endpoint with url %s", url);
- array_list_pt entries = hashMap_remove(poller->entries, url);
+ celix_array_list_t* entries = hashMap_remove(poller->entries, url);
- if (entries != NULL) {
- for (unsigned int i = arrayList_size(entries); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(entries, i - 1);
- discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(entries, i - 1);
- endpointDescription_destroy(endpoint);
- }
- arrayList_destroy(entries);
- }
+ if (entries != NULL) {
+ for (unsigned int i = celix_arrayList_size(entries); i > 0; i--) {
+ endpoint_description_t* endpoint = celix_arrayList_get(entries, i - 1);
+ discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
+ celix_arrayList_removeAt(entries, i - 1);
+ endpointDescription_destroy(endpoint);
+ }
+ celix_arrayList_destroy(entries);
+ }
- free(origKey);
- }
- status = celixThreadMutex_unlock(&poller->pollerLock);
- }
+ free(origKey);
+ }
+ status = celixThreadMutex_unlock(&poller->pollerLock);
+ }
- return status;
+ return status;
}
-
-
-
-celix_status_t endpointDiscoveryPoller_poll(endpoint_discovery_poller_t *poller, char *url, array_list_pt currentEndpoints) {
- celix_status_t status;
- array_list_pt updatedEndpoints = NULL;
-
- // create an arraylist with a custom equality test to ensure we can find endpoints properly...
- arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &updatedEndpoints);
- status = endpointDiscoveryPoller_getEndpoints(poller, url, &updatedEndpoints);
-
- if (status == CELIX_SUCCESS) {
- if (updatedEndpoints != NULL) {
- for (unsigned int i = arrayList_size(currentEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(currentEndpoints, i - 1);
-
- if (!arrayList_contains(updatedEndpoints, endpoint)) {
- status = discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(currentEndpoints, i - 1);
- endpointDescription_destroy(endpoint);
- }
- }
-
- for (int i = arrayList_size(updatedEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_remove(updatedEndpoints, 0);
-
- if (!arrayList_contains(currentEndpoints, endpoint)) {
- arrayList_add(currentEndpoints, endpoint);
- status = discovery_addDiscoveredEndpoint(poller->discovery, endpoint);
- } else {
- endpointDescription_destroy(endpoint);
-
- }
- }
- }
- }
-
- if (updatedEndpoints != NULL) {
- arrayList_destroy(updatedEndpoints);
- }
-
- return status;
+celix_status_t
+endpointDiscoveryPoller_poll(endpoint_discovery_poller_t* poller, char* url, celix_array_list_t* currentEndpoints) {
+ celix_status_t status;
+ celix_array_list_t* updatedEndpoints = NULL;
+
+ // create an arraylist with a custom equality test to ensure we can find endpoints properly...
+ updatedEndpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
+ status = endpointDiscoveryPoller_getEndpoints(poller, url, &updatedEndpoints);
+
+ if (updatedEndpoints && status == CELIX_SUCCESS) {
+ if (updatedEndpoints != NULL) {
Review Comment:
Unnecessary guard.
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
Review Comment:
It seems that we can use `inet_pton` instead of `celix_utils_convertIpToUint`.
For example, the following implementation passes all non-error-injection tests.
```C
uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
if (converted) {
*converted = false;
}
struct in_addr result;
uint32_t ipAsUint = 0;
if(inet_pton(AF_INET, ip, &result)) {
ipAsUint = ntohl(result.s_addr);
if (converted) {
*converted = true;
}
} else {
celix_err_pushf("Failed to convert IP address %s to unsigned int", ip);
}
return ipAsUint;
}
```
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
if (ret != 1) {
+ celix_err_pushf("Failed to convert netmask %s to in_addr object", netmask);
return -1;
}
// Now convert the mask to a prefix
int prefix = 0;
- bool processed_one = false;
- unsigned int i = ntohl(in.s_addr);
+ uint32_t i = ntohl(in.s_addr);
while (i > 0) {
if (i & 1) {
prefix++;
Review Comment:
Could the loop be replaced by `__builtin_popcount`, which is supported by both clang and gcc.
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
if (ret != 1) {
+ celix_err_pushf("Failed to convert netmask %s to in_addr object", netmask);
return -1;
}
// Now convert the mask to a prefix
int prefix = 0;
- bool processed_one = false;
- unsigned int i = ntohl(in.s_addr);
+ uint32_t i = ntohl(in.s_addr);
while (i > 0) {
if (i & 1) {
prefix++;
- processed_one = true;
- } else {
- if (processed_one) return -1;
}
-
i >>= 1;
}
return prefix;
}
-/** Finds an IP of the available network interfaces of the machine by specifying an CIDR subnet.
- *
- * @param ipWithPrefix IP with prefix, e.g. 192.168.1.0/24
- * @return ip In case a matching interface could be found, an allocated string containing the IP of the
- * interface will be returned, e.g. 192.168.1.16. Memory for the new string can be freed with free().
- * When no matching interface is found NULL will be returned.
- */
-char *ipUtils_findIpBySubnet(const char *ipWithPrefix) {
- char *ip = NULL;
+celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp) {
+ assert(subnetCidrNotation);
+ assert(foundIp);
+
+ *foundIp = NULL;
- char *input = celix_utils_strdup(ipWithPrefix); // Make a copy as otherwise strtok_r manipulates the input string
- if (input == NULL) {
- goto strdup_failed;
+ //create copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(subnetCidrNotation);
Review Comment:
Considering that valid CIDR notation cannot be very long, `snprintf` into a stack buffer and checking its return value should be enough. Memory allocation and error injection efforts can be avoided.
##########
bundles/remote_services/rsa_spi/include/remote_service_admin.h:
##########
@@ -41,10 +41,10 @@ typedef struct remote_service_admin remote_service_admin_t;
struct remote_service_admin_service {
remote_service_admin_t *admin;
- celix_status_t (*exportService)(remote_service_admin_t *admin, char *serviceId, celix_properties_t *properties, array_list_pt *registrations);
+ celix_status_t (*exportService)(remote_service_admin_t *admin, char *serviceId, celix_properties_t *properties, celix_array_list_t** registrations);
celix_status_t (*removeExportedService)(remote_service_admin_t *admin, export_registration_t *registration);
- celix_status_t (*getExportedServices)(remote_service_admin_t *admin, array_list_pt *services);
- celix_status_t (*getImportedEndpoints)(remote_service_admin_t *admin, array_list_pt *services);
+ celix_status_t (*getExportedServices)(remote_service_admin_t *admin, celix_array_list_t** services);
Review Comment:
Missing #include "celix_array_list.h". It depends on transitive inclusion. According to Large Scale C++ by Lakos, "transitive includes are not permitted, except to access a public base class of a type whose definition is included directly." We should include it because we need it directly, and thus should not rely on other header file.
This remark applies to multiple places of this PR.
##########
bundles/remote_services/discovery_common/src/endpoint_discovery_poller.c:
##########
@@ -308,7 +316,7 @@ static void *endpointDiscoveryPoller_performPeriodicPoll(void *data) {
hash_map_entry_pt entry = hashMapIterator_nextEntry(iterator);
char *url = hashMapEntry_getKey(entry);
- array_list_pt currentEndpoints = hashMapEntry_getValue(entry);
+ celix_array_list_t* currentEndpoints = hashMapEntry_getValue(entry);
Review Comment:
nitpick, indentation.
##########
bundles/remote_services/topology_manager/src/scope.c:
##########
@@ -120,10 +120,13 @@ celix_status_t tm_addImportScope(void *handle, char *filter) {
return CELIX_ILLEGAL_ARGUMENT; // filter not parsable
}
if (celixThreadMutex_lock(&scope->importScopeLock) == CELIX_SUCCESS) {
- int index = arrayList_indexOf(scope->importScopes, new);
- filter_pt present = (filter_pt) arrayList_get(scope->importScopes, index);
+ celix_array_list_entry_t entry;
+ memset(&entry, 0, sizeof(entry));
+ entry.voidPtrVal = new;
+ int index = celix_arrayList_indexOf(scope->importScopes, entry);
Review Comment:
I found this dubious. If the intention is to add the filter to `importScopes` if it is not there, then calling `celix_arrayList_get` on `index` that may be -1 is simply illegal, i.e., out of bound memory access. Please have a look at this, @xuzhenbao
##########
bundles/remote_services/discovery_common/src/endpoint_discovery_poller.c:
##########
@@ -183,111 +184,118 @@ celix_status_t endpointDiscoveryPoller_getDiscoveryEndpoints(endpoint_discovery_
/**
* Adds a new endpoint URL to the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status;
+celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status;
- status = celixThreadMutex_lock(&(poller)->pollerLock);
- if (status != CELIX_SUCCESS) {
- return CELIX_BUNDLE_EXCEPTION;
- }
+ status = celixThreadMutex_lock(&(poller)->pollerLock);
+ if (status != CELIX_SUCCESS) {
+ return CELIX_BUNDLE_EXCEPTION;
+ }
- // Avoid memory leaks when adding an already existing URL...
- array_list_pt endpoints = hashMap_get(poller->entries, url);
- if (endpoints == NULL) {
- status = arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &endpoints);
+ // Avoid memory leaks when adding an already existing URL...
+ celix_array_list_t* endpoints = hashMap_get(poller->entries, url);
+ if (endpoints == NULL) {
+ endpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
- if (status == CELIX_SUCCESS) {
+ if (endpoints) {
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: add new discovery endpoint with url %s", url);
- hashMap_put(poller->entries, strdup(url), endpoints);
- endpointDiscoveryPoller_poll(poller, url, endpoints);
- }
- }
+ hashMap_put(poller->entries, strdup(url), endpoints);
+ endpointDiscoveryPoller_poll(poller, url, endpoints);
+ }
+ }
- status = celixThreadMutex_unlock(&poller->pollerLock);
+ status = celixThreadMutex_unlock(&poller->pollerLock);
- return status;
+ return status;
}
/**
* Removes an endpoint URL from the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status = CELIX_SUCCESS;
-
- if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
- status = CELIX_BUNDLE_EXCEPTION;
- } else {
- hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
-
- if (entry == NULL) {
- celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?", url);
- } else {
- char* origKey = hashMapEntry_getKey(entry);
+celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status = CELIX_SUCCESS;
+
+ if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
+ status = CELIX_BUNDLE_EXCEPTION;
+ } else {
+ hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
+
+ if (entry == NULL) {
+ celix_logHelper_debug(
+ *poller->loghelper,
+ "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?",
+ url);
+ } else {
+ char* origKey = hashMapEntry_getKey(entry);
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: remove discovery endpoint with url %s", url);
- array_list_pt entries = hashMap_remove(poller->entries, url);
+ celix_array_list_t* entries = hashMap_remove(poller->entries, url);
- if (entries != NULL) {
- for (unsigned int i = arrayList_size(entries); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(entries, i - 1);
- discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(entries, i - 1);
- endpointDescription_destroy(endpoint);
- }
- arrayList_destroy(entries);
- }
+ if (entries != NULL) {
+ for (unsigned int i = celix_arrayList_size(entries); i > 0; i--) {
+ endpoint_description_t* endpoint = celix_arrayList_get(entries, i - 1);
+ discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
+ celix_arrayList_removeAt(entries, i - 1);
+ endpointDescription_destroy(endpoint);
+ }
+ celix_arrayList_destroy(entries);
+ }
- free(origKey);
- }
- status = celixThreadMutex_unlock(&poller->pollerLock);
- }
+ free(origKey);
+ }
+ status = celixThreadMutex_unlock(&poller->pollerLock);
+ }
- return status;
+ return status;
}
-
-
-
-celix_status_t endpointDiscoveryPoller_poll(endpoint_discovery_poller_t *poller, char *url, array_list_pt currentEndpoints) {
- celix_status_t status;
- array_list_pt updatedEndpoints = NULL;
-
- // create an arraylist with a custom equality test to ensure we can find endpoints properly...
- arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &updatedEndpoints);
- status = endpointDiscoveryPoller_getEndpoints(poller, url, &updatedEndpoints);
-
- if (status == CELIX_SUCCESS) {
- if (updatedEndpoints != NULL) {
- for (unsigned int i = arrayList_size(currentEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(currentEndpoints, i - 1);
-
- if (!arrayList_contains(updatedEndpoints, endpoint)) {
- status = discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(currentEndpoints, i - 1);
- endpointDescription_destroy(endpoint);
- }
- }
-
- for (int i = arrayList_size(updatedEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_remove(updatedEndpoints, 0);
-
- if (!arrayList_contains(currentEndpoints, endpoint)) {
- arrayList_add(currentEndpoints, endpoint);
- status = discovery_addDiscoveredEndpoint(poller->discovery, endpoint);
- } else {
- endpointDescription_destroy(endpoint);
-
- }
- }
- }
- }
-
- if (updatedEndpoints != NULL) {
- arrayList_destroy(updatedEndpoints);
- }
-
- return status;
+celix_status_t
+endpointDiscoveryPoller_poll(endpoint_discovery_poller_t* poller, char* url, celix_array_list_t* currentEndpoints) {
+ celix_status_t status;
+ celix_array_list_t* updatedEndpoints = NULL;
+
+ // create an arraylist with a custom equality test to ensure we can find endpoints properly...
+ updatedEndpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
+ status = endpointDiscoveryPoller_getEndpoints(poller, url, &updatedEndpoints);
+
+ if (updatedEndpoints && status == CELIX_SUCCESS) {
+ if (updatedEndpoints != NULL) {
+ for (int i = celix_arrayList_size(currentEndpoints); i > 0; i--) {
+ endpoint_description_t* endpoint = celix_arrayList_get(currentEndpoints, i - 1);
+
+ celix_array_list_entry_t entry;
+ memset(&entry, 0, sizeof(entry));
+ entry.voidPtrVal = endpoint;
+
+ if (celix_arrayList_indexOf(updatedEndpoints, entry) < 0) {
+ status = discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
+ celix_arrayList_removeAt(currentEndpoints, i - 1);
+ endpointDescription_destroy(endpoint);
+ }
+ }
+
+ for (int i = 0; i < celix_arrayList_size(updatedEndpoints); i++) {
+ endpoint_description_t* endpoint = celix_arrayList_get(updatedEndpoints, i);
+ celix_array_list_entry_t entry;
+ memset(&entry, 0, sizeof(entry));
+ entry.voidPtrVal = endpoint;
+ if (celix_arrayList_indexOf(currentEndpoints, entry) < 0) {
+ celix_arrayList_add(currentEndpoints, endpoint);
+ status = discovery_addDiscoveredEndpoint(poller->discovery, endpoint);
Review Comment:
What if `celix_arrayList_add` or `discovery_addDiscoveredEndpoint` fails? Fixing this is out the scope of this PR, and could be left for further PR. Maybe we need an issue similar to #686 for RSA.
##########
bundles/remote_services/topology_manager/src/scope.c:
##########
@@ -150,12 +153,15 @@ celix_status_t tm_removeImportScope(void *handle, char *filter) {
}
if (celixThreadMutex_lock(&scope->importScopeLock) == CELIX_SUCCESS) {
- int index = arrayList_indexOf(scope->importScopes, new);
- filter_pt present = (filter_pt) arrayList_get(scope->importScopes, index);
+ celix_array_list_entry_t entry;
+ memset(&entry, 0, sizeof(entry));
+ entry.voidPtrVal = new;
+ int index = celix_arrayList_indexOf(scope->importScopes, entry);
+ filter_pt present = (filter_pt) celix_arrayList_get(scope->importScopes, index);
if (present == NULL)
Review Comment:
Could `present` be NULL if `index != -1`?
##########
bundles/remote_services/topology_manager/tms_tst/disc_mock/disc_mock_activator.c:
##########
@@ -44,12 +44,12 @@ celix_status_t celix_bundleActivator_create(celix_bundle_context_t *context, voi
discMockService_create(act, &act->serv);
act->endpointListener = NULL;
act->endpointListenerService = NULL;
- status = arrayList_create(&act->endpointList);
+ act->endpointList = celix_arrayList_create();
} else {
status = CELIX_ENOMEM;
}
- if (status == CELIX_SUCCESS) {
+ if (status == CELIX_SUCCESS && act->endpointList) {
Review Comment:
Even if `act->endpointList == NULL`, `CELIX_SUCCESS` will be returned.
##########
bundles/shell/remote_shell/src/remote_shell.c:
##########
@@ -87,7 +92,7 @@ celix_status_t remoteShell_destroy(remote_shell_pt instance) {
remoteShell_stopConnections(instance);
celixThreadMutex_lock(&instance->mutex);
- arrayList_destroy(instance->connections);
+ celix_arrayList_destroy(instance->connections);
Review Comment:
nitpick, indentation.
##########
bundles/remote_services/discovery_common/src/endpoint_discovery_poller.c:
##########
@@ -183,111 +184,118 @@ celix_status_t endpointDiscoveryPoller_getDiscoveryEndpoints(endpoint_discovery_
/**
* Adds a new endpoint URL to the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status;
+celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status;
- status = celixThreadMutex_lock(&(poller)->pollerLock);
- if (status != CELIX_SUCCESS) {
- return CELIX_BUNDLE_EXCEPTION;
- }
+ status = celixThreadMutex_lock(&(poller)->pollerLock);
+ if (status != CELIX_SUCCESS) {
+ return CELIX_BUNDLE_EXCEPTION;
+ }
- // Avoid memory leaks when adding an already existing URL...
- array_list_pt endpoints = hashMap_get(poller->entries, url);
- if (endpoints == NULL) {
- status = arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &endpoints);
+ // Avoid memory leaks when adding an already existing URL...
+ celix_array_list_t* endpoints = hashMap_get(poller->entries, url);
+ if (endpoints == NULL) {
+ endpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
- if (status == CELIX_SUCCESS) {
+ if (endpoints) {
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: add new discovery endpoint with url %s", url);
- hashMap_put(poller->entries, strdup(url), endpoints);
- endpointDiscoveryPoller_poll(poller, url, endpoints);
- }
- }
+ hashMap_put(poller->entries, strdup(url), endpoints);
+ endpointDiscoveryPoller_poll(poller, url, endpoints);
+ }
+ }
- status = celixThreadMutex_unlock(&poller->pollerLock);
+ status = celixThreadMutex_unlock(&poller->pollerLock);
- return status;
+ return status;
}
/**
* Removes an endpoint URL from the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status = CELIX_SUCCESS;
-
- if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
- status = CELIX_BUNDLE_EXCEPTION;
- } else {
- hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
-
- if (entry == NULL) {
- celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?", url);
- } else {
- char* origKey = hashMapEntry_getKey(entry);
+celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status = CELIX_SUCCESS;
+
+ if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
+ status = CELIX_BUNDLE_EXCEPTION;
+ } else {
+ hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
+
+ if (entry == NULL) {
+ celix_logHelper_debug(
+ *poller->loghelper,
+ "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?",
+ url);
+ } else {
+ char* origKey = hashMapEntry_getKey(entry);
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: remove discovery endpoint with url %s", url);
- array_list_pt entries = hashMap_remove(poller->entries, url);
+ celix_array_list_t* entries = hashMap_remove(poller->entries, url);
- if (entries != NULL) {
- for (unsigned int i = arrayList_size(entries); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(entries, i - 1);
- discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(entries, i - 1);
- endpointDescription_destroy(endpoint);
- }
- arrayList_destroy(entries);
- }
+ if (entries != NULL) {
+ for (unsigned int i = celix_arrayList_size(entries); i > 0; i--) {
+ endpoint_description_t* endpoint = celix_arrayList_get(entries, i - 1);
+ discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
+ celix_arrayList_removeAt(entries, i - 1);
+ endpointDescription_destroy(endpoint);
+ }
+ celix_arrayList_destroy(entries);
+ }
- free(origKey);
- }
- status = celixThreadMutex_unlock(&poller->pollerLock);
- }
+ free(origKey);
+ }
+ status = celixThreadMutex_unlock(&poller->pollerLock);
+ }
- return status;
+ return status;
}
-
-
-
-celix_status_t endpointDiscoveryPoller_poll(endpoint_discovery_poller_t *poller, char *url, array_list_pt currentEndpoints) {
- celix_status_t status;
- array_list_pt updatedEndpoints = NULL;
-
- // create an arraylist with a custom equality test to ensure we can find endpoints properly...
- arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &updatedEndpoints);
- status = endpointDiscoveryPoller_getEndpoints(poller, url, &updatedEndpoints);
-
- if (status == CELIX_SUCCESS) {
- if (updatedEndpoints != NULL) {
- for (unsigned int i = arrayList_size(currentEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(currentEndpoints, i - 1);
-
- if (!arrayList_contains(updatedEndpoints, endpoint)) {
- status = discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(currentEndpoints, i - 1);
- endpointDescription_destroy(endpoint);
- }
- }
-
- for (int i = arrayList_size(updatedEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_remove(updatedEndpoints, 0);
-
- if (!arrayList_contains(currentEndpoints, endpoint)) {
- arrayList_add(currentEndpoints, endpoint);
- status = discovery_addDiscoveredEndpoint(poller->discovery, endpoint);
- } else {
- endpointDescription_destroy(endpoint);
-
- }
- }
- }
- }
-
- if (updatedEndpoints != NULL) {
- arrayList_destroy(updatedEndpoints);
- }
-
- return status;
+celix_status_t
+endpointDiscoveryPoller_poll(endpoint_discovery_poller_t* poller, char* url, celix_array_list_t* currentEndpoints) {
+ celix_status_t status;
+ celix_array_list_t* updatedEndpoints = NULL;
+
+ // create an arraylist with a custom equality test to ensure we can find endpoints properly...
+ updatedEndpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
+ status = endpointDiscoveryPoller_getEndpoints(poller, url, &updatedEndpoints);
+
+ if (updatedEndpoints && status == CELIX_SUCCESS) {
+ if (updatedEndpoints != NULL) {
+ for (int i = celix_arrayList_size(currentEndpoints); i > 0; i--) {
+ endpoint_description_t* endpoint = celix_arrayList_get(currentEndpoints, i - 1);
+
+ celix_array_list_entry_t entry;
+ memset(&entry, 0, sizeof(entry));
+ entry.voidPtrVal = endpoint;
+
+ if (celix_arrayList_indexOf(updatedEndpoints, entry) < 0) {
+ status = discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
+ celix_arrayList_removeAt(currentEndpoints, i - 1);
+ endpointDescription_destroy(endpoint);
+ }
+ }
+
+ for (int i = 0; i < celix_arrayList_size(updatedEndpoints); i++) {
+ endpoint_description_t* endpoint = celix_arrayList_get(updatedEndpoints, i);
+ celix_array_list_entry_t entry;
+ memset(&entry, 0, sizeof(entry));
+ entry.voidPtrVal = endpoint;
+ if (celix_arrayList_indexOf(currentEndpoints, entry) < 0) {
+ celix_arrayList_add(currentEndpoints, endpoint);
+ status = discovery_addDiscoveredEndpoint(poller->discovery, endpoint);
+ } else {
+ endpointDescription_destroy(endpoint);
+ }
+ }
+ celix_arrayList_clear(updatedEndpoints);
Review Comment:
Is it unnecessary if we have `celix_arrayList_destroy`?
##########
libs/framework/src/dm_dependency_manager_impl.c:
##########
@@ -367,13 +368,13 @@ celix_status_t dependencyManager_getInfo(celix_dependency_manager_t *manager, dm
celixThreadMutex_lock(&manager->mutex);
if (info != NULL) {
- arrayList_create(&info->components);
- size = arrayList_size(manager->components);
+ info->components = celix_arrayList_create();
+ size = celix_arrayList_size(manager->components);
for (i = 0; i < size; i += 1) {
- celix_dm_component_t *cmp = arrayList_get(manager->components, i);
+ celix_dm_component_t *cmp = celix_arrayList_get(manager->components, i);
cmpInfo = NULL;
component_getComponentInfo(cmp, &cmpInfo);
- arrayList_add(info->components, cmpInfo);
+ celix_arrayList_add(info->components, cmpInfo);
Review Comment:
nitpick, indentation.
##########
libs/framework/src/bundle_context_private.h:
##########
@@ -80,7 +80,7 @@ struct celix_bundle_context {
celix_bundle_t *bundle;
celix_thread_mutex_t mutex; //protects fields below (NOTE/FIXME also used by bundle.c for listing service tracker usage)
- array_list_t *svcRegistrations; //serviceIds
+ celix_array_list_t *svcRegistrations; //serviceIds
Review Comment:
nitpick, indentation.
##########
bundles/shell/remote_shell/src/remote_shell.c:
##########
@@ -204,7 +209,7 @@ void *remoteShell_connection_run(void *data) {
celix_logHelper_log(*connection->parent->loghelper, CELIX_LOG_LEVEL_INFO, "REMOTE_SHELL: Closing socket");
celixThreadMutex_lock(&connection->parent->mutex);
- arrayList_removeElement(connection->parent->connections, connection);
+ celix_arrayList_remove(connection->parent->connections, connection);
Review Comment:
nitpick, indentation.
##########
bundles/shell/remote_shell/src/remote_shell.c:
##########
@@ -61,24 +61,29 @@ static celix_status_t remoteShell_connection_print(connection_pt connection, cha
static celix_status_t remoteShell_connection_execute(connection_pt connection, char *command);
static void* remoteShell_connection_run(void *data);
-celix_status_t remoteShell_create(shell_mediator_pt mediator, int maximumConnections, remote_shell_pt *instance) {
- celix_status_t status = CELIX_SUCCESS;
- (*instance) = calloc(1, sizeof(**instance));
- if ((*instance) != NULL) {
- (*instance)->mediator = mediator;
- (*instance)->maximumConnections = maximumConnections;
- (*instance)->connections = NULL;
- (*instance)->loghelper = &mediator->loghelper;
-
- status = celixThreadMutex_create(&(*instance)->mutex, NULL);
-
- if (status == CELIX_SUCCESS) {
- status = arrayList_create(&(*instance)->connections);
- }
- } else {
- status = CELIX_ENOMEM;
- }
- return status;
+celix_status_t remoteShell_create(shell_mediator_pt mediator, int maximumConnections, remote_shell_pt* instance) {
+ celix_status_t status = CELIX_SUCCESS;
+ (*instance) = calloc(1, sizeof(**instance));
+ if ((*instance) != NULL) {
+ (*instance)->mediator = mediator;
+ (*instance)->maximumConnections = maximumConnections;
+ (*instance)->connections = NULL;
+ (*instance)->loghelper = &mediator->loghelper;
+
+ status = celixThreadMutex_create(&(*instance)->mutex, NULL);
+
+ if (status == CELIX_SUCCESS) {
+ (*instance)->connections = celix_arrayList_create();
+ if (!(*instance)->connections) {
Review Comment:
`mutex` is not destroyed. celix_cleanup and early return will be handy.
##########
bundles/shell/remote_shell/src/remote_shell.c:
##########
@@ -106,9 +111,9 @@ celix_status_t remoteShell_addConnection(remote_shell_pt instance, int socket) {
celixThreadMutex_lock(&instance->mutex);
- if (arrayList_size(instance->connections) < instance->maximumConnections) {
+ if (celix_arrayList_size(instance->connections) < instance->maximumConnections) {
celix_thread_t connectionRunThread = celix_thread_default;
- arrayList_add(instance->connections, connection);
+ celix_arrayList_add(instance->connections, connection);
Review Comment:
nitpick, indentation.
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
Review Comment:
Can this be replaced by `inet_ntop`?
##########
bundles/remote_services/discovery_common/src/endpoint_discovery_poller.c:
##########
@@ -183,111 +184,118 @@ celix_status_t endpointDiscoveryPoller_getDiscoveryEndpoints(endpoint_discovery_
/**
* Adds a new endpoint URL to the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status;
+celix_status_t endpointDiscoveryPoller_addDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status;
- status = celixThreadMutex_lock(&(poller)->pollerLock);
- if (status != CELIX_SUCCESS) {
- return CELIX_BUNDLE_EXCEPTION;
- }
+ status = celixThreadMutex_lock(&(poller)->pollerLock);
+ if (status != CELIX_SUCCESS) {
+ return CELIX_BUNDLE_EXCEPTION;
+ }
- // Avoid memory leaks when adding an already existing URL...
- array_list_pt endpoints = hashMap_get(poller->entries, url);
- if (endpoints == NULL) {
- status = arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &endpoints);
+ // Avoid memory leaks when adding an already existing URL...
+ celix_array_list_t* endpoints = hashMap_get(poller->entries, url);
+ if (endpoints == NULL) {
+ endpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
- if (status == CELIX_SUCCESS) {
+ if (endpoints) {
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: add new discovery endpoint with url %s", url);
- hashMap_put(poller->entries, strdup(url), endpoints);
- endpointDiscoveryPoller_poll(poller, url, endpoints);
- }
- }
+ hashMap_put(poller->entries, strdup(url), endpoints);
+ endpointDiscoveryPoller_poll(poller, url, endpoints);
+ }
+ }
- status = celixThreadMutex_unlock(&poller->pollerLock);
+ status = celixThreadMutex_unlock(&poller->pollerLock);
- return status;
+ return status;
}
/**
* Removes an endpoint URL from the list of polled endpoints.
*/
-celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t *poller, char *url) {
- celix_status_t status = CELIX_SUCCESS;
-
- if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
- status = CELIX_BUNDLE_EXCEPTION;
- } else {
- hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
-
- if (entry == NULL) {
- celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?", url);
- } else {
- char* origKey = hashMapEntry_getKey(entry);
+celix_status_t endpointDiscoveryPoller_removeDiscoveryEndpoint(endpoint_discovery_poller_t* poller, char* url) {
+ celix_status_t status = CELIX_SUCCESS;
+
+ if (celixThreadMutex_lock(&poller->pollerLock) != CELIX_SUCCESS) {
+ status = CELIX_BUNDLE_EXCEPTION;
+ } else {
+ hash_map_entry_pt entry = hashMap_getEntry(poller->entries, url);
+
+ if (entry == NULL) {
+ celix_logHelper_debug(
+ *poller->loghelper,
+ "ENDPOINT_POLLER: There was no entry found belonging to url %s - maybe already removed?",
+ url);
+ } else {
+ char* origKey = hashMapEntry_getKey(entry);
celix_logHelper_debug(*poller->loghelper, "ENDPOINT_POLLER: remove discovery endpoint with url %s", url);
- array_list_pt entries = hashMap_remove(poller->entries, url);
+ celix_array_list_t* entries = hashMap_remove(poller->entries, url);
- if (entries != NULL) {
- for (unsigned int i = arrayList_size(entries); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(entries, i - 1);
- discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(entries, i - 1);
- endpointDescription_destroy(endpoint);
- }
- arrayList_destroy(entries);
- }
+ if (entries != NULL) {
+ for (unsigned int i = celix_arrayList_size(entries); i > 0; i--) {
+ endpoint_description_t* endpoint = celix_arrayList_get(entries, i - 1);
+ discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
+ celix_arrayList_removeAt(entries, i - 1);
+ endpointDescription_destroy(endpoint);
+ }
+ celix_arrayList_destroy(entries);
+ }
- free(origKey);
- }
- status = celixThreadMutex_unlock(&poller->pollerLock);
- }
+ free(origKey);
+ }
+ status = celixThreadMutex_unlock(&poller->pollerLock);
+ }
- return status;
+ return status;
}
-
-
-
-celix_status_t endpointDiscoveryPoller_poll(endpoint_discovery_poller_t *poller, char *url, array_list_pt currentEndpoints) {
- celix_status_t status;
- array_list_pt updatedEndpoints = NULL;
-
- // create an arraylist with a custom equality test to ensure we can find endpoints properly...
- arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals, &updatedEndpoints);
- status = endpointDiscoveryPoller_getEndpoints(poller, url, &updatedEndpoints);
-
- if (status == CELIX_SUCCESS) {
- if (updatedEndpoints != NULL) {
- for (unsigned int i = arrayList_size(currentEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_get(currentEndpoints, i - 1);
-
- if (!arrayList_contains(updatedEndpoints, endpoint)) {
- status = discovery_removeDiscoveredEndpoint(poller->discovery, endpoint);
- arrayList_remove(currentEndpoints, i - 1);
- endpointDescription_destroy(endpoint);
- }
- }
-
- for (int i = arrayList_size(updatedEndpoints); i > 0; i--) {
- endpoint_description_t *endpoint = arrayList_remove(updatedEndpoints, 0);
-
- if (!arrayList_contains(currentEndpoints, endpoint)) {
- arrayList_add(currentEndpoints, endpoint);
- status = discovery_addDiscoveredEndpoint(poller->discovery, endpoint);
- } else {
- endpointDescription_destroy(endpoint);
-
- }
- }
- }
- }
-
- if (updatedEndpoints != NULL) {
- arrayList_destroy(updatedEndpoints);
- }
-
- return status;
+celix_status_t
+endpointDiscoveryPoller_poll(endpoint_discovery_poller_t* poller, char* url, celix_array_list_t* currentEndpoints) {
+ celix_status_t status;
+ celix_array_list_t* updatedEndpoints = NULL;
+
+ // create an arraylist with a custom equality test to ensure we can find endpoints properly...
+ updatedEndpoints = celix_arrayList_createWithEquals(endpointDiscoveryPoller_endpointDescriptionEquals);
+ status = endpointDiscoveryPoller_getEndpoints(poller, url, &updatedEndpoints);
+
+ if (updatedEndpoints && status == CELIX_SUCCESS) {
+ if (updatedEndpoints != NULL) {
+ for (int i = celix_arrayList_size(currentEndpoints); i > 0; i--) {
+ endpoint_description_t* endpoint = celix_arrayList_get(currentEndpoints, i - 1);
+
+ celix_array_list_entry_t entry;
+ memset(&entry, 0, sizeof(entry));
Review Comment:
`celix_array_list_entry_t entry = {0};` may be better than `memset`?
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
Review Comment:
Will 0xFFFFFFFF make more sense for prefix > 32?
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
Review Comment:
We need to verify that `netmask` is indeed a valid ipv4 network mask.
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
Review Comment:
It seems to me that `uint32_t netmask` makes more sense than `const char* netmask`.
Moreover, for uint32_t, some extra bit operations are enough to verify it is indeed a valid ipv4 network mask. For string form mask, it is not that easy to do that verification.
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
if (ret != 1) {
+ celix_err_pushf("Failed to convert netmask %s to in_addr object", netmask);
return -1;
}
// Now convert the mask to a prefix
int prefix = 0;
- bool processed_one = false;
- unsigned int i = ntohl(in.s_addr);
+ uint32_t i = ntohl(in.s_addr);
while (i > 0) {
if (i & 1) {
prefix++;
- processed_one = true;
- } else {
- if (processed_one) return -1;
}
-
i >>= 1;
}
return prefix;
}
-/** Finds an IP of the available network interfaces of the machine by specifying an CIDR subnet.
- *
- * @param ipWithPrefix IP with prefix, e.g. 192.168.1.0/24
- * @return ip In case a matching interface could be found, an allocated string containing the IP of the
- * interface will be returned, e.g. 192.168.1.16. Memory for the new string can be freed with free().
- * When no matching interface is found NULL will be returned.
- */
-char *ipUtils_findIpBySubnet(const char *ipWithPrefix) {
- char *ip = NULL;
+celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp) {
+ assert(subnetCidrNotation);
+ assert(foundIp);
+
+ *foundIp = NULL;
- char *input = celix_utils_strdup(ipWithPrefix); // Make a copy as otherwise strtok_r manipulates the input string
- if (input == NULL) {
- goto strdup_failed;
+ //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 CELIX_ENOMEM;
}
- char *savePtr;
- char *inputIp = strtok_r(input, "/", &savePtr);
- char *inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int inputPrefix = (unsigned int) strtoul(inputPrefixStr, NULL, 10);
+ char* savePtr;
+ char* inputIp = strtok_r(input, "/", &savePtr);
+ char* inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int ipAsUint = ipUtils_ipToUnsignedInt(inputIp);
- unsigned int bitmask = ipUtils_prefixToBitmask(inputPrefix);
+ if (!inputPrefixStr) {
+ celix_err_pushf("Failed to parse IP address with prefix %s. Missing a '/'", subnetCidrNotation);
+ 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 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 CELIX_ILLEGAL_ARGUMENT;
+ }
+
+ bool converted;
+ uint32_t ipAsUint = celix_utils_convertIpToUint(inputIp, &converted);
+ if (!converted) {
+ return CELIX_ILLEGAL_ARGUMENT;
+ }
+ uint32_t bitmask = celix_utils_ipPrefixLengthToBitmask(inputPrefix);
unsigned int ipRangeStart = ipAsUint & bitmask;
unsigned int ipRangeStop = ipAsUint | ~bitmask;
// Requested IP range is known now, now loop through network interfaces
struct ifaddrs *ifap, *ifa;
- if(getifaddrs (&ifap) == -1) {
- goto getifaddrs_failed;
+ if (getifaddrs(&ifap) == -1) {
+ celix_status_t status = errno;
+ celix_err_push("Failed to get network interfaces");
+ return status;
}
+
for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
- if (ifa->ifa_addr == NULL)
+ if (ifa->ifa_addr == NULL) {
continue;
+ }
- if (ifa->ifa_addr->sa_family != AF_INET)
+ if (ifa->ifa_addr->sa_family != AF_INET) {
continue;
+ }
// Retrieve IP address for interface
char if_addr[NI_MAXHOST];
- int rv = getnameinfo(ifa->ifa_addr, sizeof(struct sockaddr_in),
- if_addr, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
+ int rv = getnameinfo(ifa->ifa_addr, sizeof(struct sockaddr_in), if_addr, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
if (rv != 0) {
- printf("getnameinfo() failed: %s\n", gai_strerror(rv));
+ celix_err_pushf("Failed to get IP address for interface %s: %s", ifa->ifa_name, gai_strerror(rv));
continue;
}
// Retrieve netmask
- struct sockaddr_in *sa = (struct sockaddr_in *) ifa->ifa_netmask;
- char *if_netmask = inet_ntoa(sa->sin_addr);
+ struct sockaddr_in* sa = (struct sockaddr_in*)ifa->ifa_netmask;
+ char* if_netmask = inet_ntoa(sa->sin_addr);
Review Comment:
`inet_ntoa` uses statically allocated buffer, and thus IMO should be avoided.
Moreover, if `celix_utils_ipNetmaskToPrefixLength` accepts `uint32_t` rather than `const char*`, then this is unnecessary.
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
if (ret != 1) {
+ celix_err_pushf("Failed to convert netmask %s to in_addr object", netmask);
return -1;
}
// Now convert the mask to a prefix
int prefix = 0;
- bool processed_one = false;
- unsigned int i = ntohl(in.s_addr);
+ uint32_t i = ntohl(in.s_addr);
while (i > 0) {
if (i & 1) {
prefix++;
- processed_one = true;
- } else {
- if (processed_one) return -1;
}
-
i >>= 1;
}
return prefix;
}
-/** Finds an IP of the available network interfaces of the machine by specifying an CIDR subnet.
- *
- * @param ipWithPrefix IP with prefix, e.g. 192.168.1.0/24
- * @return ip In case a matching interface could be found, an allocated string containing the IP of the
- * interface will be returned, e.g. 192.168.1.16. Memory for the new string can be freed with free().
- * When no matching interface is found NULL will be returned.
- */
-char *ipUtils_findIpBySubnet(const char *ipWithPrefix) {
- char *ip = NULL;
+celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp) {
+ assert(subnetCidrNotation);
+ assert(foundIp);
+
+ *foundIp = NULL;
- char *input = celix_utils_strdup(ipWithPrefix); // Make a copy as otherwise strtok_r manipulates the input string
- if (input == NULL) {
- goto strdup_failed;
+ //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 CELIX_ENOMEM;
}
- char *savePtr;
- char *inputIp = strtok_r(input, "/", &savePtr);
- char *inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int inputPrefix = (unsigned int) strtoul(inputPrefixStr, NULL, 10);
+ char* savePtr;
+ char* inputIp = strtok_r(input, "/", &savePtr);
+ char* inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int ipAsUint = ipUtils_ipToUnsignedInt(inputIp);
- unsigned int bitmask = ipUtils_prefixToBitmask(inputPrefix);
+ if (!inputPrefixStr) {
+ celix_err_pushf("Failed to parse IP address with prefix %s. Missing a '/'", subnetCidrNotation);
Review Comment:
Missing a '/' may not be accurate. For exmaple,
```C++
EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_utils_findIpInSubnet("/", &ipAddr)); // empty address
EXPECT_EQ(1, celix_err_getErrorCount());
```
##########
libs/utils/src/ip_utils.c:
##########
@@ -16,155 +16,207 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * ip_utils.c
- *
- * \date Jun 24, 2019
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
-#include "ip_utils.h"
+#include "celix_ip_utils.h"
+
+#include "celix_err.h"
+#include "celix_stdlib_cleanup.h"
#include "celix_utils.h"
+#include "celix_convert_utils.h"
+#include <arpa/inet.h>
+#include <assert.h>
#include <errno.h>
+#include <ifaddrs.h>
+#include <limits.h>
+#include <math.h>
+#include <netdb.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
-#include <stdbool.h>
#include <string.h>
-#include <math.h>
-
-#include <arpa/inet.h>
#include <sys/socket.h>
-#include <netdb.h>
-#include <ifaddrs.h>
-#include <unistd.h>
-unsigned int ipUtils_ipToUnsignedInt(char *ip) {
- unsigned int ipAsUint = 0;
+uint32_t celix_utils_convertIpToUint(const char* ip, bool* converted) {
+ if (converted) {
+ *converted = false;
+ }
- char *partOfIp = NULL, *savePtr = NULL;
- char *input = strdup(ip); // Make a copy because otherwise strtok_r manipulates the input string
- partOfIp = strtok_r(input, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 3);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 2);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 1);
- partOfIp = strtok_r(NULL, ".\0", &savePtr); ipAsUint += strtoul(partOfIp, NULL, 10) * (unsigned int) pow(256, 0);
- free(input);
+ // copy for strtok_r
+ celix_autofree char* input = celix_utils_strdup(ip);
+ if (!input) {
+ celix_err_push("Failed to duplicate input string for IP conversion");
+ return 0;
+ }
+ uint32_t ipAsUint = 0;
+ char* partOfIp = NULL;
+ char* savePtr = NULL;
+ int count = 0;
+ while ((partOfIp = strtok_r(partOfIp == NULL ? input : NULL, ".\0", &savePtr)) != NULL) {
+ if (count > 3) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, to many parts", ip);
+ return 0;
+ }
+ bool longConverted = false;
+ long partAsLong = celix_utils_convertStringToLong(partOfIp, ULONG_MAX, &longConverted);
+ if (!longConverted) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is not a number", ip, partOfIp);
+ return 0;
+ } else if (partAsLong > 255 || partAsLong < 0) {
+ celix_err_pushf("Failed to convert IP address %s to unsigned int, part `%s` is out of range", ip, partOfIp);
+ return 0;
+ }
+ ipAsUint += (uint32_t)(partAsLong * powl(256, 3 - count++));
+ }
+
+ if (converted) {
+ *converted = true;
+ }
return ipAsUint;
}
-char *ipUtils_unsignedIntToIp(unsigned int ip) {
- char *ipStr = calloc(16, sizeof(char));
+char* celix_utils_convertUintToIp(uint32_t ip) {
+ char* ipStr = calloc(16, sizeof(char));
+ if (!ipStr) {
+ celix_err_push("Failed to allocate memory for IP address string");
+ return NULL;
+ }
- int ipPart1 = ip / (int) pow(256, 3); ip -= ipPart1 * (int) pow(256, 3);
- int ipPart2 = ip / (int) pow(256, 2); ip -= ipPart2 * (int) pow(256, 2);
- int ipPart3 = ip / (int) pow(256, 1); ip -= ipPart3 * (int) pow(256, 1);
- int ipPart4 = ip / (int) pow(256, 0);
+ int64_t ipPart1 = ip / (int64_t)pow(256, 3);
+ ip -= ipPart1 * (int64_t)pow(256, 3);
+ int64_t ipPart2 = ip / (int64_t)pow(256, 2);
+ ip -= ipPart2 * (int64_t)pow(256, 2);
+ int64_t ipPart3 = ip / (int64_t)pow(256, 1);
+ ip -= ipPart3 * (int64_t)pow(256, 1);
+ int64_t ipPart4 = ip / (int64_t)pow(256, 0);
- snprintf(ipStr, 16, "%d.%d.%d.%d", ipPart1, ipPart2, ipPart3, ipPart4);
+ snprintf(ipStr, 16, "%i.%i.%i.%i", (int)ipPart1, (int)ipPart2, (int)ipPart3, (int)ipPart4);
return ipStr;
}
-unsigned int ipUtils_prefixToBitmask(unsigned int prefix) {
- return (0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF;
+uint32_t celix_utils_ipPrefixLengthToBitmask(int prefix) {
+ if (prefix > 32 || prefix <= 0) {
+ return 0;
+ }
+ return (uint32_t )((0xFFFFFFFF << (32 - prefix)) & 0xFFFFFFFF);
}
-int ipUtils_netmaskToPrefix(const char *netmask) {
+int celix_utils_ipNetmaskToPrefixLength(const char* netmask) {
// Convert netmask to in_addr object
struct in_addr in;
int ret = inet_pton(AF_INET, netmask, &in);
if (ret != 1) {
+ celix_err_pushf("Failed to convert netmask %s to in_addr object", netmask);
return -1;
}
// Now convert the mask to a prefix
int prefix = 0;
- bool processed_one = false;
- unsigned int i = ntohl(in.s_addr);
+ uint32_t i = ntohl(in.s_addr);
while (i > 0) {
if (i & 1) {
prefix++;
- processed_one = true;
- } else {
- if (processed_one) return -1;
}
-
i >>= 1;
}
return prefix;
}
-/** Finds an IP of the available network interfaces of the machine by specifying an CIDR subnet.
- *
- * @param ipWithPrefix IP with prefix, e.g. 192.168.1.0/24
- * @return ip In case a matching interface could be found, an allocated string containing the IP of the
- * interface will be returned, e.g. 192.168.1.16. Memory for the new string can be freed with free().
- * When no matching interface is found NULL will be returned.
- */
-char *ipUtils_findIpBySubnet(const char *ipWithPrefix) {
- char *ip = NULL;
+celix_status_t celix_utils_findIpInSubnet(const char* subnetCidrNotation, char** foundIp) {
+ assert(subnetCidrNotation);
+ assert(foundIp);
+
+ *foundIp = NULL;
- char *input = celix_utils_strdup(ipWithPrefix); // Make a copy as otherwise strtok_r manipulates the input string
- if (input == NULL) {
- goto strdup_failed;
+ //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 CELIX_ENOMEM;
}
- char *savePtr;
- char *inputIp = strtok_r(input, "/", &savePtr);
- char *inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int inputPrefix = (unsigned int) strtoul(inputPrefixStr, NULL, 10);
+ char* savePtr;
+ char* inputIp = strtok_r(input, "/", &savePtr);
+ char* inputPrefixStr = strtok_r(NULL, "\0", &savePtr);
- unsigned int ipAsUint = ipUtils_ipToUnsignedInt(inputIp);
- unsigned int bitmask = ipUtils_prefixToBitmask(inputPrefix);
+ if (!inputPrefixStr) {
+ celix_err_pushf("Failed to parse IP address with prefix %s. Missing a '/'", subnetCidrNotation);
+ 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 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 CELIX_ILLEGAL_ARGUMENT;
+ }
+
+ bool converted;
+ uint32_t ipAsUint = celix_utils_convertIpToUint(inputIp, &converted);
+ if (!converted) {
+ return CELIX_ILLEGAL_ARGUMENT;
+ }
+ uint32_t bitmask = celix_utils_ipPrefixLengthToBitmask(inputPrefix);
unsigned int ipRangeStart = ipAsUint & bitmask;
unsigned int ipRangeStop = ipAsUint | ~bitmask;
// Requested IP range is known now, now loop through network interfaces
struct ifaddrs *ifap, *ifa;
- if(getifaddrs (&ifap) == -1) {
- goto getifaddrs_failed;
+ if (getifaddrs(&ifap) == -1) {
+ celix_status_t status = errno;
+ celix_err_push("Failed to get network interfaces");
+ return status;
}
+
for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
- if (ifa->ifa_addr == NULL)
+ if (ifa->ifa_addr == NULL) {
continue;
+ }
- if (ifa->ifa_addr->sa_family != AF_INET)
+ if (ifa->ifa_addr->sa_family != AF_INET) {
continue;
+ }
// Retrieve IP address for interface
char if_addr[NI_MAXHOST];
- int rv = getnameinfo(ifa->ifa_addr, sizeof(struct sockaddr_in),
- if_addr, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
+ int rv = getnameinfo(ifa->ifa_addr, sizeof(struct sockaddr_in), if_addr, NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
Review Comment:
We already have `ifa->ifa_addr` ipv4 address, why bother calling `getnameinfo`?
The following `celix_utils_convertIpToUint` seems unnecessary.
##########
libs/utils/src/hash_map_private.h:
##########
@@ -29,13 +29,17 @@
#include "hash_map.h"
-unsigned int hashMap_hashCode(const void* toHash);
-int hashMap_equals(const void* toCompare, const void* compare);
+#ifdef __cplusplus
+extern "C" {
+#endif
-void hashMap_resize(hash_map_pt map, int newCapacity);
-hash_map_entry_pt hashMap_removeEntryForKey(hash_map_pt map, const void* key);
-hash_map_entry_pt hashMap_removeMapping(hash_map_pt map, hash_map_entry_pt entry);
-void hashMap_addEntry(hash_map_pt map, int hash, void* key, void* value, int bucketIndex);
+CELIX_UTILS_DEPRECATED_EXPORT unsigned int hashMap_hashCode(const void* toHash);
Review Comment:
Why export? `CELIX_UTILS_DEPRECATED` seems more reasonable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/509 remove cpputests (celix)
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes merged PR #711:
URL: https://github.com/apache/celix/pull/711
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/509 remove cpputests (celix)
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on PR #711:
URL: https://github.com/apache/celix/pull/711#issuecomment-1876862694
> The current `ip_utils` is not very satisfatctory. In addition to the drawbacks mentioned below, the current implementation lacks support for ipv6. Considering IPv6 link-local address combined with Bonjour provides real zero configuration networking experience, IPv6 support is critical for IOT devices.
Good point, then I will remove the ip utils in this branch. If this is needed again the ip utils in commit id `1a67dabc5b87f0c8250ce4351df310873be3b0b9` can be restored.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Feature/509 remove cpputests (celix)
Posted by "pnoltes (via GitHub)" <gi...@apache.org>.
pnoltes commented on PR #711:
URL: https://github.com/apache/celix/pull/711#issuecomment-1877137267
> > I also think this should be addressed in a separate ticket, if you agree I will make a ticket for this.
>
> Yes, we can address this later.
Created #715 for this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org