[lxc-devel] [lxc/master] tree-wide: fixes and cleanups

brauner on Github lxc-bot at linuxcontainers.org
Tue Dec 8 11:19:59 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201208/eb7b5217/attachment-0001.bin>
-------------- next part --------------
From c4ef8f4c1103c87144e5dabe051d23b3619179d7 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 8 Dec 2020 11:53:54 +0100
Subject: [PATCH 1/3] tree-wide: use call_cleaner(netns_freeifaddrs)

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/include/netns_ifaddrs.h |  3 +-
 src/lxc/confile.c           | 12 +++----
 src/lxc/lxccontainer.c      | 72 +++++++++++++++++--------------------
 3 files changed, 38 insertions(+), 49 deletions(-)

diff --git a/src/include/netns_ifaddrs.h b/src/include/netns_ifaddrs.h
index 1b8703ee7d..d3f1d6eef8 100644
--- a/src/include/netns_ifaddrs.h
+++ b/src/include/netns_ifaddrs.h
@@ -13,7 +13,7 @@ extern "C" {
 #include <sys/socket.h>
 
 #include "compiler.h"
-#include "netns_ifaddrs.h"
+#include "memory_utils.h"
 
 struct netns_ifaddrs {
 	struct netns_ifaddrs *ifa_next;
@@ -52,6 +52,7 @@ struct netns_ifaddrs {
 #define __ifa_dstaddr ifa_ifu.ifu_dstaddr
 
 __hidden extern void netns_freeifaddrs(struct netns_ifaddrs *);
+define_cleanup_function(struct netns_ifaddrs *, netns_freeifaddrs);
 __hidden extern int netns_getifaddrs(struct netns_ifaddrs **ifap, __s32 netns_id,
 				     bool *netnsid_aware);
 
diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 4f7621a900..6f5bf3909b 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -377,17 +377,16 @@ static int set_config_net_flags(const char *key, const char *value,
 static int create_matched_ifnames(const char *value, struct lxc_conf *lxc_conf,
 				  struct lxc_netdev *netdev)
 {
-	struct netns_ifaddrs *ifaddr, *ifa;
+	call_cleaner(netns_freeifaddrs) struct netns_ifaddrs *ifaddr = NULL;
+	struct netns_ifaddrs *ifa;
 	int n;
 	int ret = 0;
 	const char *type_key = "lxc.net.type";
 	const char *link_key = "lxc.net.link";
 	const char *tmpvalue = "phys";
 
-	if (netns_getifaddrs(&ifaddr, -1, &(bool){false}) < 0) {
-		SYSERROR("Failed to get network interfaces");
-		return -1;
-	}
+	if (netns_getifaddrs(&ifaddr, -1, &(bool){false}) < 0)
+		return log_error_errno(-1, errno, "Failed to get network interfaces");
 
 	for (ifa = ifaddr, n = 0; ifa != NULL; ifa = ifa->ifa_next, n++) {
 		if (!ifa->ifa_addr)
@@ -413,9 +412,6 @@ static int create_matched_ifnames(const char *value, struct lxc_conf *lxc_conf,
 		}
 	}
 
-	netns_freeifaddrs(ifaddr);
-	ifaddr = NULL;
-
 	return ret;
 }
 
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 96aa372e1d..da18be8aa1 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -2340,20 +2340,21 @@ static char **do_lxcapi_get_interfaces(struct lxc_container *c)
 	char **interfaces = NULL;
 	char interface[IFNAMSIZ];
 
-	if (pipe2(pipefd, O_CLOEXEC) < 0)
-		return NULL;
+	if (pipe2(pipefd, O_CLOEXEC))
+		return log_error_errno(NULL, errno, "Failed to create pipe");
 
 	pid = fork();
 	if (pid < 0) {
-		SYSERROR("Failed to fork task to get interfaces information");
 		close(pipefd[0]);
 		close(pipefd[1]);
-		return NULL;
+		return log_error_errno(NULL, errno, "Failed to fork task to get interfaces information");
 	}
 
-	if (pid == 0) { /* child */
-		int ret = 1, nbytes;
-		struct netns_ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;
+	if (pid == 0) {
+		call_cleaner(netns_freeifaddrs) struct netns_ifaddrs *ifaddrs = NULL;
+		struct netns_ifaddrs *ifa = NULL;
+		int ret = 1;
+		int nbytes;
 
 		/* close the read-end of the pipe */
 		close(pipefd[0]);
@@ -2364,15 +2365,15 @@ static char **do_lxcapi_get_interfaces(struct lxc_container *c)
 		}
 
 		/* Grab the list of interfaces */
-		if (netns_getifaddrs(&interfaceArray, -1, &(bool){false})) {
+		if (netns_getifaddrs(&ifaddrs, -1, &(bool){false})) {
 			SYSERROR("Failed to get interfaces list");
 			goto out;
 		}
 
 		/* Iterate through the interfaces */
-		for (tempIfAddr = interfaceArray; tempIfAddr != NULL;
-		     tempIfAddr = tempIfAddr->ifa_next) {
-			nbytes = lxc_write_nointr(pipefd[1], tempIfAddr->ifa_name, IFNAMSIZ);
+		for (ifa = ifaddrs; ifa != NULL;
+		     ifa = ifa->ifa_next) {
+			nbytes = lxc_write_nointr(pipefd[1], ifa->ifa_name, IFNAMSIZ);
 			if (nbytes < 0)
 				goto out;
 
@@ -2382,9 +2383,6 @@ static char **do_lxcapi_get_interfaces(struct lxc_container *c)
 		ret = 0;
 
 	out:
-		if (interfaceArray)
-			netns_freeifaddrs(interfaceArray);
-
 		/* close the write-end of the pipe, thus sending EOF to the reader */
 		close(pipefd[1]);
 		_exit(ret);
@@ -2405,7 +2403,7 @@ static char **do_lxcapi_get_interfaces(struct lxc_container *c)
 		count++;
 	}
 
-	if (wait_for_pid(pid) != 0) {
+	if (wait_for_pid(pid)) {
 		for (i = 0; i < count; i++)
 			free(interfaces[i]);
 
@@ -2436,10 +2434,8 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface,
 	char **addresses = NULL;
 
 	ret = pipe2(pipefd, O_CLOEXEC);
-	if (ret < 0) {
-		SYSERROR("Failed to create pipe");
-		return NULL;
-	}
+	if (ret < 0)
+		return log_error_errno(NULL, errno, "Failed to create pipe");
 
 	pid = fork();
 	if (pid < 0) {
@@ -2450,11 +2446,12 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface,
 	}
 
 	if (pid == 0) {
+		call_cleaner(netns_freeifaddrs) struct netns_ifaddrs *ifaddrs = NULL;
+		struct netns_ifaddrs *ifa = NULL;
 		ssize_t nbytes;
 		char addressOutputBuffer[INET6_ADDRSTRLEN];
 		char *address_ptr = NULL;
-		void *tempAddrPtr = NULL;
-		struct netns_ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL;
+		void *address_ptr_tmp = NULL;
 
 		/* close the read-end of the pipe */
 		close(pipefd[0]);
@@ -2465,52 +2462,50 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface,
 		}
 
 		/* Grab the list of interfaces */
-		if (netns_getifaddrs(&interfaceArray, -1, &(bool){false})) {
+		if (netns_getifaddrs(&ifaddrs, -1, &(bool){false})) {
 			SYSERROR("Failed to get interfaces list");
 			goto out;
 		}
 
 		/* Iterate through the interfaces */
-		for (tempIfAddr = interfaceArray; tempIfAddr;
-		     tempIfAddr = tempIfAddr->ifa_next) {
-			if (tempIfAddr->ifa_addr == NULL)
+		for (ifa = ifaddrs; ifa; ifa = ifa->ifa_next) {
+			if (ifa->ifa_addr == NULL)
 				continue;
 
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wcast-align"
 
-			if (tempIfAddr->ifa_addr->sa_family == AF_INET) {
+			if (ifa->ifa_addr->sa_family == AF_INET) {
 				if (family && strcmp(family, "inet"))
 					continue;
 
-				tempAddrPtr = &((struct sockaddr_in *)tempIfAddr->ifa_addr)->sin_addr;
+				address_ptr_tmp = &((struct sockaddr_in *)ifa->ifa_addr)->sin_addr;
 			} else {
 				if (family && strcmp(family, "inet6"))
 					continue;
 
-				if (((struct sockaddr_in6 *)tempIfAddr->ifa_addr)->sin6_scope_id != scope)
+				if (((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_scope_id != scope)
 					continue;
 
-				tempAddrPtr = &((struct sockaddr_in6 *)tempIfAddr->ifa_addr)->sin6_addr;
+				address_ptr_tmp = &((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_addr;
 			}
 
 #pragma GCC diagnostic pop
 
-			if (interface && strcmp(interface, tempIfAddr->ifa_name))
+			if (interface && strcmp(interface, ifa->ifa_name))
 				continue;
-			else if (!interface && strcmp("lo", tempIfAddr->ifa_name) == 0)
+			else if (!interface && strcmp("lo", ifa->ifa_name) == 0)
 				continue;
 
-			address_ptr = (char *)inet_ntop(tempIfAddr->ifa_addr->sa_family,
-						    tempAddrPtr, addressOutputBuffer,
-						    sizeof(addressOutputBuffer));
+			address_ptr = (char *)inet_ntop(ifa->ifa_addr->sa_family, address_ptr_tmp,
+							addressOutputBuffer,
+							sizeof(addressOutputBuffer));
 			if (!address_ptr)
 				continue;
 
 			nbytes = lxc_write_nointr(pipefd[1], address_ptr, INET6_ADDRSTRLEN);
 			if (nbytes != INET6_ADDRSTRLEN) {
-				SYSERROR("Failed to send ipv6 address \"%s\"",
-					 address_ptr);
+				SYSERROR("Failed to send ipv6 address \"%s\"", address_ptr);
 				goto out;
 			}
 
@@ -2520,9 +2515,6 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface,
 		ret = 0;
 
 	out:
-		if (interfaceArray)
-			netns_freeifaddrs(interfaceArray);
-
 		/* close the write-end of the pipe, thus sending EOF to the reader */
 		close(pipefd[1]);
 		_exit(ret);
@@ -2540,7 +2532,7 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface,
 		count++;
 	}
 
-	if (wait_for_pid(pid) != 0) {
+	if (wait_for_pid(pid)) {
 		for (i = 0; i < count; i++)
 			free(addresses[i]);
 

From 059a1ec30bf9a04c296b173b72c143a720176af2 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 8 Dec 2020 12:05:47 +0100
Subject: [PATCH 2/3] confile: clean up network configuration parsing

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/confile.c | 273 ++++++++++++++++++++--------------------------
 src/lxc/utils.c   |   4 +-
 2 files changed, 120 insertions(+), 157 deletions(-)

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 6f5bf3909b..90b52f7a41 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -323,7 +323,7 @@ static int set_config_net_type(const char *key, const char *value,
 		return clr_config_net_type(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	if (strcmp(value, "veth") == 0) {
 		netdev->type = LXC_NET_VETH;
@@ -351,8 +351,7 @@ static int set_config_net_type(const char *key, const char *value,
 	} else if (strcmp(value, "none") == 0) {
 		netdev->type = LXC_NET_NONE;
 	} else {
-		ERROR("Invalid network type %s", value);
-		return -1;
+		return log_error(-1, "Invalid network type %s", value);
 	}
 
 	return 0;
@@ -367,7 +366,7 @@ static int set_config_net_flags(const char *key, const char *value,
 		return clr_config_net_flags(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	netdev->flags |= IFF_UP;
 
@@ -425,7 +424,7 @@ static int set_config_net_link(const char *key, const char *value,
 		return clr_config_net_link(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	if (value[strlen(value) - 1] == '+' && netdev->type == LXC_NET_PHYS)
 		ret = create_matched_ifnames(value, lxc_conf, netdev);
@@ -446,11 +445,11 @@ static int set_config_net_l2proxy(const char *key, const char *value,
 		return clr_config_net_l2proxy(key, lxc_conf, data);
 
 	if (!netdev)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
 	ret = lxc_safe_uint(value, &val);
 	if (ret < 0)
-		return ret_set_errno(-1, -ret);
+		return ret_errno(-ret);
 
 	switch (val) {
 	case 0:
@@ -473,7 +472,7 @@ static int set_config_net_name(const char *key, const char *value,
 		return clr_config_net_name(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	return network_ifname(netdev->name, value, sizeof(netdev->name));
 }
@@ -488,7 +487,7 @@ static int set_config_net_veth_mode(const char *key, const char *value,
 		return clr_config_net_veth_mode(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	return lxc_veth_mode_to_flag(&netdev->priv.veth_attr.mode, value);
 }
@@ -502,9 +501,10 @@ static int set_config_net_veth_pair(const char *key, const char *value,
 		return clr_config_net_veth_pair(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
-	return network_ifname(netdev->priv.veth_attr.pair, value, sizeof(netdev->priv.veth_attr.pair));
+	return network_ifname(netdev->priv.veth_attr.pair, value,
+			      sizeof(netdev->priv.veth_attr.pair));
 }
 
 static int set_config_net_veth_vlan_id(const char *key, const char *value,
@@ -579,7 +579,7 @@ static int set_config_net_macvlan_mode(const char *key, const char *value,
 		return clr_config_net_macvlan_mode(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	return lxc_macvlan_mode_to_flag(&netdev->priv.macvlan_attr.mode, value);
 }
@@ -593,12 +593,12 @@ static int set_config_net_ipvlan_mode(const char *key, const char *value,
 		return clr_config_net_ipvlan_mode(key, lxc_conf, data);
 
 	if (!netdev)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
-	if (netdev->type != LXC_NET_IPVLAN) {
-		SYSERROR("Invalid ipvlan mode \"%s\", can only be used with ipvlan network", value);
-		return ret_set_errno(-1, EINVAL);
-	}
+	if (netdev->type != LXC_NET_IPVLAN)
+		return log_error_errno(-EINVAL,
+				       EINVAL, "Invalid ipvlan mode \"%s\", can only be used with ipvlan network",
+				       value);
 
 	return lxc_ipvlan_mode_to_flag(&netdev->priv.ipvlan_attr.mode, value);
 }
@@ -612,12 +612,12 @@ static int set_config_net_ipvlan_isolation(const char *key, const char *value,
 		return clr_config_net_ipvlan_isolation(key, lxc_conf, data);
 
 	if (!netdev)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
-	if (netdev->type != LXC_NET_IPVLAN) {
-		SYSERROR("Invalid ipvlan isolation \"%s\", can only be used with ipvlan network", value);
-		return ret_set_errno(-1, EINVAL);
-	}
+	if (netdev->type != LXC_NET_IPVLAN)
+		return log_error_errno(-EINVAL,
+				       EINVAL, "Invalid ipvlan isolation \"%s\", can only be used with ipvlan network",
+				       value);
 
 	return lxc_ipvlan_isolation_to_flag(&netdev->priv.ipvlan_attr.isolation, value);
 }
@@ -632,11 +632,11 @@ static int set_config_net_hwaddr(const char *key, const char *value,
 		return clr_config_net_hwaddr(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	new_value = strdup(value);
 	if (!new_value)
-		return -1;
+		return ret_errno(ENOMEM);
 
 	rand_complete_hwaddr(new_value);
 
@@ -661,11 +661,11 @@ static int set_config_net_vlan_id(const char *key, const char *value,
 		return clr_config_net_vlan_id(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	ret = get_u16(&netdev->priv.vlan_attr.vid, value, 0);
 	if (ret < 0)
-		return -1;
+		return ret;
 
 	return 0;
 }
@@ -679,7 +679,7 @@ static int set_config_net_mtu(const char *key, const char *value,
 		return clr_config_net_mtu(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	return set_config_string_item(&netdev->mtu, value);
 }
@@ -687,39 +687,34 @@ static int set_config_net_mtu(const char *key, const char *value,
 static int set_config_net_ipv4_address(const char *key, const char *value,
 				       struct lxc_conf *lxc_conf, void *data)
 {
+	__do_free char *addr = NULL;
+	__do_free struct lxc_inetdev *inetdev = NULL;
+	__do_free struct lxc_list *list = NULL;
 	int ret;
 	struct lxc_netdev *netdev = data;
-	struct lxc_inetdev *inetdev;
-	struct lxc_list *list;
 	char *cursor, *slash;
-	char *addr = NULL, *bcast = NULL, *prefix = NULL;
+	char *bcast = NULL, *prefix = NULL;
 
 	if (lxc_config_value_empty(value))
 		return clr_config_net_ipv4_address(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	inetdev = malloc(sizeof(*inetdev));
 	if (!inetdev)
-		return -1;
+		return ret_errno(ENOMEM);
 	memset(inetdev, 0, sizeof(*inetdev));
 
 	list = malloc(sizeof(*list));
-	if (!list) {
-		free(inetdev);
-		return -1;
-	}
+	if (!list)
+		return ret_errno(ENOMEM);
 
 	lxc_list_init(list);
-	list->elem = inetdev;
 
 	addr = strdup(value);
-	if (!addr) {
-		free(inetdev);
-		free(list);
-		return -1;
-	}
+	if (!addr)
+		return ret_errno(ENOMEM);
 
 	cursor = strstr(addr, " ");
 	if (cursor) {
@@ -734,35 +729,21 @@ static int set_config_net_ipv4_address(const char *key, const char *value,
 	}
 
 	ret = inet_pton(AF_INET, addr, &inetdev->addr);
-	if (!ret || ret < 0) {
-		SYSERROR("Invalid ipv4 address \"%s\"", value);
-		free(inetdev);
-		free(addr);
-		free(list);
-		return -1;
-	}
+	if (!ret || ret < 0)
+		return log_error_errno(-1, errno, "Invalid ipv4 address \"%s\"", value);
 
 	if (bcast) {
 		ret = inet_pton(AF_INET, bcast, &inetdev->bcast);
-		if (!ret || ret < 0) {
-			SYSERROR("Invalid ipv4 broadcast address \"%s\"", value);
-			free(inetdev);
-			free(list);
-			free(addr);
-			return -1;
-		}
+		if (!ret || ret < 0)
+			return log_error_errno(-1, errno, "Invalid ipv4 broadcast address \"%s\"", value);
 
 	}
 
 	/* No prefix specified, determine it from the network class. */
 	if (prefix) {
 		ret = lxc_safe_uint(prefix, &inetdev->prefix);
-		if (ret < 0) {
-			free(inetdev);
-			free(list);
-			free(addr);
-			return -1;
-		}
+		if (ret < 0)
+			return ret;
 	} else {
 		inetdev->prefix = config_ip_prefix(&inetdev->addr);
 	}
@@ -775,8 +756,10 @@ static int set_config_net_ipv4_address(const char *key, const char *value,
 		inetdev->bcast.s_addr |= htonl(INADDR_BROADCAST >> inetdev->prefix);
 	}
 
+	list->elem = inetdev;
 	lxc_list_add_tail(&netdev->ipv4, list);
-	free(addr);
+	move_ptr(inetdev);
+	move_ptr(list);
 
 	return 0;
 }
@@ -802,21 +785,18 @@ static int set_config_net_ipv4_gateway(const char *key, const char *value,
 		netdev->ipv4_gateway_auto = false;
 		netdev->ipv4_gateway_dev = true;
 	} else {
+		__do_free struct in_addr *gw = NULL;
 		int ret;
-		struct in_addr *gw;
 
 		gw = malloc(sizeof(*gw));
 		if (!gw)
-			return -1;
+			return ret_errno(ENOMEM);
 
 		ret = inet_pton(AF_INET, value, gw);
-		if (!ret || ret < 0) {
-			SYSERROR("Invalid ipv4 gateway address \"%s\"", value);
-			free(gw);
-			return -1;
-		}
+		if (!ret || ret < 0)
+			return log_error_errno(-1, errno, "Invalid ipv4 gateway address \"%s\"", value);
 
-		netdev->ipv4_gateway = gw;
+		netdev->ipv4_gateway = move_ptr(gw);
 		netdev->ipv4_gateway_auto = false;
 	}
 
@@ -837,47 +817,47 @@ static int set_config_net_veth_ipv4_route(const char *key, const char *value,
 		return clr_config_net_veth_ipv4_route(key, lxc_conf, data);
 
 	if (!netdev)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
-	if (netdev->type != LXC_NET_VETH) {
-		SYSERROR("Invalid ipv4 route \"%s\", can only be used with veth network", value);
-		return ret_set_errno(-1, EINVAL);
-	}
+	if (netdev->type != LXC_NET_VETH)
+		return log_error_errno(-EINVAL,
+				       EINVAL, "Invalid ipv4 route \"%s\", can only be used with veth network",
+				       value);
 
 	inetdev = malloc(sizeof(*inetdev));
 	if (!inetdev)
-		return -1;
+		return ret_errno(ENOMEM);
 	memset(inetdev, 0, sizeof(*inetdev));
 
 	list = malloc(sizeof(*list));
 	if (!list)
-		return -1;
+		return ret_errno(ENOMEM);
 
 	lxc_list_init(list);
 	list->elem = inetdev;
 
 	valdup = strdup(value);
 	if (!valdup)
-		return -1;
+		return ret_errno(ENOMEM);
 
 	slash = strchr(valdup, '/');
 	if (!slash)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
 	*slash = '\0';
 	slash++;
 	if (*slash == '\0')
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
 	netmask = slash;
 
 	ret = lxc_safe_uint(netmask, &inetdev->prefix);
 	if (ret < 0 || inetdev->prefix > 32)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
 	ret = inet_pton(AF_INET, valdup, &inetdev->addr);
 	if (!ret || ret < 0)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
 	lxc_list_add_tail(&netdev->priv.veth_attr.ipv4_routes, list);
 	move_ptr(inetdev);
@@ -889,38 +869,33 @@ static int set_config_net_veth_ipv4_route(const char *key, const char *value,
 static int set_config_net_ipv6_address(const char *key, const char *value,
 				       struct lxc_conf *lxc_conf, void *data)
 {
+	__do_free char *valdup = NULL;
+	__do_free struct lxc_inet6dev *inet6dev = NULL;
+	__do_free struct lxc_list *list = NULL;
 	int ret;
 	struct lxc_netdev *netdev = data;
-	struct lxc_inet6dev *inet6dev;
-	struct lxc_list *list;
-	char *slash, *valdup, *netmask;
+	char *slash, *netmask;
 
 	if (lxc_config_value_empty(value))
 		return clr_config_net_ipv6_address(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	inet6dev = malloc(sizeof(*inet6dev));
 	if (!inet6dev)
-		return -1;
+		return ret_errno(ENOMEM);
 	memset(inet6dev, 0, sizeof(*inet6dev));
 
 	list = malloc(sizeof(*list));
-	if (!list) {
-		free(inet6dev);
-		return -1;
-	}
+	if (!list)
+		return ret_errno(ENOMEM);
 
 	lxc_list_init(list);
-	list->elem = inet6dev;
 
 	valdup = strdup(value);
-	if (!valdup) {
-		free(list);
-		free(inet6dev);
-		return -1;
-	}
+	if (!valdup)
+		return ret_errno(ENOMEM);
 
 	inet6dev->prefix = 64;
 	slash = strstr(valdup, "/");
@@ -929,25 +904,18 @@ static int set_config_net_ipv6_address(const char *key, const char *value,
 		netmask = slash + 1;
 
 		ret = lxc_safe_uint(netmask, &inet6dev->prefix);
-		if (ret < 0) {
-			free(list);
-			free(inet6dev);
-			free(valdup);
-			return -1;
-		}
+		if (ret < 0)
+			return ret;
 	}
 
 	ret = inet_pton(AF_INET6, valdup, &inet6dev->addr);
-	if (!ret || ret < 0) {
-		SYSERROR("Invalid ipv6 address \"%s\"", valdup);
-		free(list);
-		free(inet6dev);
-		free(valdup);
-		return -1;
-	}
+	if (!ret || ret < 0)
+		return log_error_errno(-EINVAL, EINVAL, "Invalid ipv6 address \"%s\"", valdup);
 
+	list->elem = inet6dev;
 	lxc_list_add_tail(&netdev->ipv6, list);
-	free(valdup);
+	move_ptr(inet6dev);
+	move_ptr(list);
 
 	return 0;
 }
@@ -961,7 +929,7 @@ static int set_config_net_ipv6_gateway(const char *key, const char *value,
 		return clr_config_net_ipv6_gateway(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	free(netdev->ipv6_gateway);
 
@@ -974,20 +942,18 @@ static int set_config_net_ipv6_gateway(const char *key, const char *value,
 		netdev->ipv6_gateway_dev = true;
 	} else {
 		int ret;
-		struct in6_addr *gw;
+		__do_free struct in6_addr *gw = NULL;
 
 		gw = malloc(sizeof(*gw));
 		if (!gw)
-			return -1;
+			return ret_errno(ENOMEM);
 
 		ret = inet_pton(AF_INET6, value, gw);
-		if (!ret || ret < 0) {
-			SYSERROR("Invalid ipv6 gateway address \"%s\"", value);
-			free(gw);
-			return -1;
-		}
+		if (!ret || ret < 0)
+			return log_error_errno(-EINVAL, EINVAL,
+					       "Invalid ipv6 gateway address \"%s\"", value);
 
-		netdev->ipv6_gateway = gw;
+		netdev->ipv6_gateway = move_ptr(gw);
 		netdev->ipv6_gateway_auto = false;
 	}
 
@@ -1008,24 +974,23 @@ static int set_config_net_veth_ipv6_route(const char *key, const char *value,
 		return clr_config_net_veth_ipv6_route(key, lxc_conf, data);
 
 	if (!netdev)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
-	if (netdev->type != LXC_NET_VETH) {
-		SYSERROR("Invalid ipv6 route \"%s\", can only be used with veth network", value);
-		return ret_set_errno(-1, EINVAL);
-	}
+	if (netdev->type != LXC_NET_VETH)
+		return log_error_errno(-EINVAL,
+				       EINVAL, "Invalid ipv6 route \"%s\", can only be used with veth network",
+				       value);
 
 	inet6dev = malloc(sizeof(*inet6dev));
 	if (!inet6dev)
-		return -1;
+		return ret_errno(ENOMEM);
 	memset(inet6dev, 0, sizeof(*inet6dev));
 
 	list = malloc(sizeof(*list));
 	if (!list)
-		return -1;
+		return ret_errno(ENOMEM);
 
 	lxc_list_init(list);
-	list->elem = inet6dev;
 
 	valdup = strdup(value);
 	if (!valdup)
@@ -1033,23 +998,24 @@ static int set_config_net_veth_ipv6_route(const char *key, const char *value,
 
 	slash = strchr(valdup, '/');
 	if (!slash)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
 	*slash = '\0';
 	slash++;
 	if (*slash == '\0')
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
 	netmask = slash;
 
 	ret = lxc_safe_uint(netmask, &inet6dev->prefix);
 	if (ret < 0 || inet6dev->prefix > 128)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
 	ret = inet_pton(AF_INET6, valdup, &inet6dev->addr);
 	if (!ret || ret < 0)
-		return ret_set_errno(-1, EINVAL);
+		return ret_errno(EINVAL);
 
+	list->elem = inet6dev;
 	lxc_list_add_tail(&netdev->priv.veth_attr.ipv6_routes, list);
 	move_ptr(inet6dev);
 	move_ptr(list);
@@ -1066,7 +1032,7 @@ static int set_config_net_script_up(const char *key, const char *value,
 		return clr_config_net_script_up(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	return set_config_string_item(&netdev->upscript, value);
 }
@@ -1080,22 +1046,21 @@ static int set_config_net_script_down(const char *key, const char *value,
 		return clr_config_net_script_down(key, lxc_conf, data);
 
 	if (!netdev)
-		return -1;
+		return ret_errno(EINVAL);
 
 	return set_config_string_item(&netdev->downscript, value);
 }
 
-static int add_hook(struct lxc_conf *lxc_conf, int which, char *hook)
+static int add_hook(struct lxc_conf *lxc_conf, int which, __owns char *hook)
 {
+	__do_free char *val = hook;
 	struct lxc_list *hooklist;
 
 	hooklist = malloc(sizeof(*hooklist));
-	if (!hooklist) {
-		free(hook);
-		return -1;
-	}
+	if (!hooklist)
+		return ret_errno(ENOMEM);
 
-	hooklist->elem = hook;
+	hooklist->elem = move_ptr(val);
 	lxc_list_add_tail(&lxc_conf->hooks[which], hooklist);
 
 	return 0;
@@ -1216,7 +1181,7 @@ static int set_config_init_gid(const char *key, const char *value,
 static int set_config_hooks(const char *key, const char *value,
 			    struct lxc_conf *lxc_conf, void *data)
 {
-	char *copy;
+	__do_free char *copy = NULL;
 
 	if (lxc_config_value_empty(value))
 		return lxc_clear_hooks(lxc_conf, key);
@@ -1228,30 +1193,28 @@ static int set_config_hooks(const char *key, const char *value,
 
 	copy = strdup(value);
 	if (!copy)
-		return -1;
+		return ret_errno(ENOMEM);
 
 	if (strcmp(key + 9, "pre-start") == 0)
-		return add_hook(lxc_conf, LXCHOOK_PRESTART, copy);
+		return add_hook(lxc_conf, LXCHOOK_PRESTART, move_ptr(copy));
 	else if (strcmp(key + 9, "start-host") == 0)
-		return add_hook(lxc_conf, LXCHOOK_START_HOST, copy);
+		return add_hook(lxc_conf, LXCHOOK_START_HOST, move_ptr(copy));
 	else if (strcmp(key + 9, "pre-mount") == 0)
-		return add_hook(lxc_conf, LXCHOOK_PREMOUNT, copy);
+		return add_hook(lxc_conf, LXCHOOK_PREMOUNT, move_ptr(copy));
 	else if (strcmp(key + 9, "autodev") == 0)
-		return add_hook(lxc_conf, LXCHOOK_AUTODEV, copy);
+		return add_hook(lxc_conf, LXCHOOK_AUTODEV, move_ptr(copy));
 	else if (strcmp(key + 9, "mount") == 0)
-		return add_hook(lxc_conf, LXCHOOK_MOUNT, copy);
+		return add_hook(lxc_conf, LXCHOOK_MOUNT, move_ptr(copy));
 	else if (strcmp(key + 9, "start") == 0)
-		return add_hook(lxc_conf, LXCHOOK_START, copy);
+		return add_hook(lxc_conf, LXCHOOK_START, move_ptr(copy));
 	else if (strcmp(key + 9, "stop") == 0)
-		return add_hook(lxc_conf, LXCHOOK_STOP, copy);
+		return add_hook(lxc_conf, LXCHOOK_STOP, move_ptr(copy));
 	else if (strcmp(key + 9, "post-stop") == 0)
-		return add_hook(lxc_conf, LXCHOOK_POSTSTOP, copy);
+		return add_hook(lxc_conf, LXCHOOK_POSTSTOP, move_ptr(copy));
 	else if (strcmp(key + 9, "clone") == 0)
-		return add_hook(lxc_conf, LXCHOOK_CLONE, copy);
+		return add_hook(lxc_conf, LXCHOOK_CLONE, move_ptr(copy));
 	else if (strcmp(key + 9, "destroy") == 0)
-		return add_hook(lxc_conf, LXCHOOK_DESTROY, copy);
-
-	free(copy);
+		return add_hook(lxc_conf, LXCHOOK_DESTROY, move_ptr(copy));
 
 	return -1;
 }
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index fdf437ef35..27b0fb7359 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -201,12 +201,12 @@ extern int get_u16(unsigned short *val, const char *arg, int base)
 	char *ptr;
 
 	if (!arg || !*arg)
-		return -1;
+		return ret_errno(EINVAL);
 
 	errno = 0;
 	res = strtoul(arg, &ptr, base);
 	if (!ptr || ptr == arg || *ptr || res > 0xFFFF || errno != 0)
-		return -1;
+		return ret_errno(ERANGE);
 
 	*val = res;
 

From ed1454e85281cbabe6821b370decaee26b919dc3 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 8 Dec 2020 12:19:04 +0100
Subject: [PATCH 3/3] confile: clean up hooks

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/confile.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 90b52f7a41..e7ab359291 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -1186,10 +1186,8 @@ static int set_config_hooks(const char *key, const char *value,
 	if (lxc_config_value_empty(value))
 		return lxc_clear_hooks(lxc_conf, key);
 
-	if (strcmp(key + 4, "hook") == 0) {
-		ERROR("lxc.hook must not have a value");
-		return -1;
-	}
+	if (strcmp(key + 4, "hook") == 0)
+		return log_error_errno(-EINVAL, EINVAL, "lxc.hook must not have a value");
 
 	copy = strdup(value);
 	if (!copy)
@@ -1232,11 +1230,9 @@ static int set_config_hooks_version(const char *key, const char *value,
 	if (ret < 0)
 		return -1;
 
-	if (tmp > 1) {
-		ERROR("Invalid hook version specified. Currently only 0 "
-		      "(legacy) and 1 are supported");
-		return -1;
-	}
+	if (tmp > 1)
+		return log_error_errno(-EINVAL,
+				       EINVAL, "Invalid hook version specified. Currently only 0 (legacy) and 1 are supported");
 
 	lxc_conf->hooks_version = tmp;
 


More information about the lxc-devel mailing list