[lxc-devel] [lxc/master] Some redundancy codes of abstract unix socket are removed with log cleanups.

2xsec on Github lxc-bot at linuxcontainers.org
Fri Oct 12 06:37:45 UTC 2018


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 404 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20181012/4e16290d/attachment.bin>
-------------- next part --------------
From 6dd32d35f9bbc39cc727fe32f7f7e5872ea4120d Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.jeong at samsung.com>
Date: Fri, 12 Oct 2018 10:36:42 +0900
Subject: [PATCH 1/3] monitor: log cleanups

Signed-off-by: 2xsec <dh48.jeong at samsung.com>
---
 src/lxc/monitor.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index 4a5b9a985..ef21457c3 100644
--- a/src/lxc/monitor.c
+++ b/src/lxc/monitor.c
@@ -73,7 +73,7 @@ int lxc_monitor_fifo_name(const char *lxcpath, char *fifo_path, size_t fifo_path
 	if (do_mkdirp) {
 		ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s", rundir, lxcpath);
 		if (ret < 0 || (size_t)ret >= fifo_path_sz) {
-			ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo.", rundir, lxcpath);
+			ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath);
 			free(rundir);
 			return -1;
 		}
@@ -86,7 +86,7 @@ int lxc_monitor_fifo_name(const char *lxcpath, char *fifo_path, size_t fifo_path
 	}
 	ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s/monitor-fifo", rundir, lxcpath);
 	if (ret < 0 || (size_t)ret >= fifo_path_sz) {
-		ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo.", rundir, lxcpath);
+		ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath);
 		free(rundir);
 		return -1;
 	}
@@ -128,7 +128,7 @@ static void lxc_monitor_fifo_send(struct lxc_msg *msg, const char *lxcpath)
 	ret = lxc_write_nointr(fd, msg, sizeof(*msg));
 	if (ret != sizeof(*msg)) {
 		close(fd);
-		SYSERROR("Failed to write to monitor fifo \"%s\".", fifo_path);
+		SYSERROR("Failed to write to monitor fifo \"%s\"", fifo_path);
 		return;
 	}
 
@@ -185,7 +185,7 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) {
 	path = alloca(len);
 	ret = snprintf(path, len, "lxc/%s/monitor-sock", lxcpath);
 	if (ret < 0 || (size_t)ret >= len) {
-		ERROR("failed to create name for monitor socket");
+		ERROR("Failed to create name for monitor socket");
 		return -1;
 	}
 
@@ -198,13 +198,13 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) {
 	hash = fnv_64a_buf(path, ret, FNV1A_64_INIT);
 	ret = snprintf(addr->sun_path, len, "@lxc/%016" PRIx64 "/%s", hash, lxcpath);
 	if (ret < 0) {
-		ERROR("failed to create hashed name for monitor socket");
+		ERROR("Failed to create hashed name for monitor socket");
 		return -1;
 	}
 
 	/* replace @ with \0 */
 	addr->sun_path[0] = '\0';
-	INFO("using monitor socket name \"%s\" (length of socket name %zu must be <= %zu)", &addr->sun_path[1], strlen(&addr->sun_path[1]), sizeof(addr->sun_path) - 3);
+	INFO("Using monitor socket name \"%s\" (length of socket name %zu must be <= %zu)", &addr->sun_path[1], strlen(&addr->sun_path[1]), sizeof(addr->sun_path) - 3);
 
 	return 0;
 }
@@ -221,7 +221,7 @@ int lxc_monitor_open(const char *lxcpath)
 		return -1;
 
 	len = strlen(&addr.sun_path[1]);
-	DEBUG("opening monitor socket %s with len %zu", &addr.sun_path[1], len);
+	DEBUG("Opening monitor socket %s with len %zu", &addr.sun_path[1], len);
 	if (len >= sizeof(addr.sun_path) - 1) {
 		errno = ENAMETOOLONG;
 		SYSERROR("The name of monitor socket too long (%zu bytes)", len);
@@ -272,7 +272,7 @@ int lxc_monitor_read_fdset(struct pollfd *fds, nfds_t nfds, struct lxc_msg *msg,
 		}
 	}
 
-	SYSERROR("No ready fd found.");
+	SYSERROR("No ready fd found");
 
 	return -1;
 }
@@ -315,33 +315,33 @@ int lxc_monitord_spawn(const char *lxcpath)
 	/* double fork to avoid zombies when monitord exits */
 	pid1 = fork();
 	if (pid1 < 0) {
-		SYSERROR("Failed to fork().");
+		SYSERROR("Failed to fork()");
 		return -1;
 	}
 
 	if (pid1) {
-		DEBUG("Going to wait for pid %d.", pid1);
+		DEBUG("Going to wait for pid %d", pid1);
 
 		if (waitpid(pid1, NULL, 0) != pid1)
 			return -1;
 
-		DEBUG("Finished waiting on pid %d.", pid1);
+		DEBUG("Finished waiting on pid %d", pid1);
 		return 0;
 	}
 
 	if (pipe(pipefd) < 0) {
-		SYSERROR("Failed to create pipe.");
+		SYSERROR("Failed to create pipe");
 		_exit(EXIT_FAILURE);
 	}
 
 	pid2 = fork();
 	if (pid2 < 0) {
-		SYSERROR("Failed to fork().");
+		SYSERROR("Failed to fork()");
 		_exit(EXIT_FAILURE);
 	}
 
 	if (pid2) {
-		DEBUG("Trying to sync with child process.");
+		DEBUG("Trying to sync with child process");
 		char c;
 		/* Wait for daemon to create socket. */
 		close(pipefd[1]);
@@ -356,18 +356,18 @@ int lxc_monitord_spawn(const char *lxcpath)
 
 		close(pipefd[0]);
 
-		DEBUG("Successfully synced with child process.");
+		DEBUG("Successfully synced with child process");
 		_exit(EXIT_SUCCESS);
 	}
 
 	if (setsid() < 0) {
-		SYSERROR("Failed to setsid().");
+		SYSERROR("Failed to setsid()");
 		_exit(EXIT_FAILURE);
 	}
 
 	lxc_check_inherited(NULL, true, &pipefd[1], 1);
 	if (null_stdfds() < 0) {
-		SYSERROR("Failed to dup2() standard file descriptors to /dev/null.");
+		SYSERROR("Failed to dup2() standard file descriptors to /dev/null");
 		_exit(EXIT_FAILURE);
 	}
 
@@ -375,14 +375,14 @@ int lxc_monitord_spawn(const char *lxcpath)
 
 	ret = snprintf(pipefd_str, sizeof(pipefd_str), "%d", pipefd[1]);
 	if (ret < 0 || ret >= sizeof(pipefd_str)) {
-		ERROR("Failed to create pid argument to pass to monitord.");
+		ERROR("Failed to create pid argument to pass to monitord");
 		_exit(EXIT_FAILURE);
 	}
 
-	DEBUG("Using pipe file descriptor %d for monitord.", pipefd[1]);
+	DEBUG("Using pipe file descriptor %d for monitord", pipefd[1]);
 
 	execvp(args[0], args);
-	SYSERROR("failed to exec lxc-monitord");
+	SYSERROR("Failed to exec lxc-monitord");
 
 	_exit(EXIT_FAILURE);
 }

From 2f1264995f17ddb206164b31c1b2b222e4913f3a Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.jeong at samsung.com>
Date: Fri, 12 Oct 2018 11:19:04 +0900
Subject: [PATCH 2/3] monitor: checking name too long to make monitor sock name

Signed-off-by: 2xsec <dh48.jeong at samsung.com>
---
 src/lxc/monitor.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index ef21457c3..64dd3511e 100644
--- a/src/lxc/monitor.c
+++ b/src/lxc/monitor.c
@@ -199,7 +199,11 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) {
 	ret = snprintf(addr->sun_path, len, "@lxc/%016" PRIx64 "/%s", hash, lxcpath);
 	if (ret < 0) {
 		ERROR("Failed to create hashed name for monitor socket");
-		return -1;
+		goto on_error;
+	} else if ((size_t)ret >= len) {
+		errno = ENAMETOOLONG;
+		SYSERROR("The name of monitor socket too long (%d bytes)", ret);
+		goto on_error;
 	}
 
 	/* replace @ with \0 */
@@ -207,6 +211,9 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) {
 	INFO("Using monitor socket name \"%s\" (length of socket name %zu must be <= %zu)", &addr->sun_path[1], strlen(&addr->sun_path[1]), sizeof(addr->sun_path) - 3);
 
 	return 0;
+
+on_error:
+	return -1;
 }
 
 int lxc_monitor_open(const char *lxcpath)
@@ -214,19 +221,12 @@ int lxc_monitor_open(const char *lxcpath)
 	struct sockaddr_un addr;
 	int fd;
 	size_t retry;
-	size_t len;
 	int backoff_ms[] = {10, 50, 100};
 
 	if (lxc_monitor_sock_name(lxcpath, &addr) < 0)
 		return -1;
 
-	len = strlen(&addr.sun_path[1]);
-	DEBUG("Opening monitor socket %s with len %zu", &addr.sun_path[1], len);
-	if (len >= sizeof(addr.sun_path) - 1) {
-		errno = ENAMETOOLONG;
-		SYSERROR("The name of monitor socket too long (%zu bytes)", len);
-		return -1;
-	}
+	DEBUG("Opening monitor socket %s with len %zu", &addr.sun_path[1], strlen(&addr.sun_path[1]));
 
 	for (retry = 0; retry < sizeof(backoff_ms) / sizeof(backoff_ms[0]); retry++) {
 		fd = lxc_abstract_unix_connect(addr.sun_path);

From 5b46db1a63dd0b663cce2cb8015c2a5aa05453a1 Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.jeong at samsung.com>
Date: Fri, 12 Oct 2018 15:05:43 +0900
Subject: [PATCH 3/3] commands_utils: improve code redundancy to make abstract
 unix socket name

Signed-off-by: 2xsec <dh48.jeong at samsung.com>
---
 src/lxc/commands.c       | 15 ++++-----------
 src/lxc/commands_utils.c | 33 ++++++++++++++++++++-------------
 src/lxc/commands_utils.h |  3 ++-
 3 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 4a8c24a37..ffa8c3537 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -1244,24 +1244,17 @@ static int lxc_cmd_accept(int fd, uint32_t events, void *data,
 
 int lxc_cmd_init(const char *name, const char *lxcpath, const char *suffix)
 {
-	int fd, len, ret;
+	int fd, ret;
 	char path[LXC_AUDS_ADDR_LEN] = {0};
-	char *offset = &path[1];
 
-	/* -2 here because this is an abstract unix socket so it needs a
-	 * leading \0, and we null terminate, so it needs a trailing \0.
-	 * Although null termination isn't required by the API, we do it anyway
-	 * because we print the sockname out sometimes.
-	 */
-	len = sizeof(path) - 2;
-	ret = lxc_make_abstract_socket_name(offset, len, name, lxcpath, NULL, suffix);
+	ret = lxc_make_abstract_socket_name(path, sizeof(path), name, lxcpath, NULL, suffix);
 	if (ret < 0)
 		return -1;
-	TRACE("Creating abstract unix socket \"%s\"", offset);
+	TRACE("Creating abstract unix socket \"%s\"", &path[1]);
 
 	fd = lxc_abstract_unix_open(path, SOCK_STREAM, 0);
 	if (fd < 0) {
-		SYSERROR("Failed to create command socket %s", offset);
+		SYSERROR("Failed to create command socket %s", &path[1]);
 		if (errno == EADDRINUSE)
 			ERROR("Container \"%s\" appears to be already running", name);
 
diff --git a/src/lxc/commands_utils.c b/src/lxc/commands_utils.c
index 5d5f55f79..812f5ceeb 100644
--- a/src/lxc/commands_utils.c
+++ b/src/lxc/commands_utils.c
@@ -96,24 +96,38 @@ int lxc_cmd_sock_get_state(const char *name, const char *lxcpath,
 	return ret;
 }
 
-int lxc_make_abstract_socket_name(char *path, int len, const char *lxcname,
+int lxc_make_abstract_socket_name(char *path, size_t pathlen,
+				  const char *lxcname,
 				  const char *lxcpath,
 				  const char *hashed_sock_name,
 				  const char *suffix)
 {
 	const char *name;
+	char *offset;
 	char *tmppath;
+	size_t len;
 	size_t tmplen;
 	uint64_t hash;
 	int ret;
 
+	if (!path)
+		return -1;
+
+	offset = &path[1];
+
+	/* -2 here because this is an abstract unix socket so it needs a
+	 * leading \0, and we null terminate, so it needs a trailing \0.
+	 * Although null termination isn't required by the API, we do it anyway
+	 * because we print the sockname out sometimes.
+	 */
+	len = pathlen -2;
+
 	name = lxcname;
 	if (!name)
 		name = "";
 
 	if (hashed_sock_name != NULL) {
-		ret =
-		    snprintf(path, len, "lxc/%s/%s", hashed_sock_name, suffix);
+		ret = snprintf(offset, len, "lxc/%s/%s", hashed_sock_name, suffix);
 		if (ret < 0 || ret >= len) {
 			ERROR("Failed to create abstract socket name");
 			return -1;
@@ -129,7 +143,7 @@ int lxc_make_abstract_socket_name(char *path, int len, const char *lxcname,
 		}
 	}
 
-	ret = snprintf(path, len, "%s/%s/%s", lxcpath, name, suffix);
+	ret = snprintf(offset, len, "%s/%s/%s", lxcpath, name, suffix);
 	if (ret < 0) {
 		ERROR("Failed to create abstract socket name");
 		return -1;
@@ -147,7 +161,7 @@ int lxc_make_abstract_socket_name(char *path, int len, const char *lxcname,
 	}
 
 	hash = fnv_64a_buf(tmppath, ret, FNV1A_64_INIT);
-	ret = snprintf(path, len, "lxc/%016" PRIx64 "/%s", hash, suffix);
+	ret = snprintf(offset, len, "lxc/%016" PRIx64 "/%s", hash, suffix);
 	if (ret < 0 || ret >= len) {
 		ERROR("Failed to create abstract socket name");
 		return -1;
@@ -161,15 +175,8 @@ int lxc_cmd_connect(const char *name, const char *lxcpath,
 {
 	int ret, client_fd;
 	char path[LXC_AUDS_ADDR_LEN] = {0};
-	char *offset = &path[1];
 
-	/* -2 here because this is an abstract unix socket so it needs a
-	 * leading \0, and we null terminate, so it needs a trailing \0.
-	 * Although null termination isn't required by the API, we do it anyway
-	 * because we print the sockname out sometimes.
-	 */
-	size_t len = sizeof(path) - 2;
-	ret = lxc_make_abstract_socket_name(offset, len, name, lxcpath,
+	ret = lxc_make_abstract_socket_name(path, sizeof(path), name, lxcpath,
 					    hashed_sock_name, suffix);
 	if (ret < 0)
 		return -1;
diff --git a/src/lxc/commands_utils.h b/src/lxc/commands_utils.h
index d54cb11f3..c1583b785 100644
--- a/src/lxc/commands_utils.h
+++ b/src/lxc/commands_utils.h
@@ -25,7 +25,8 @@
 #include "state.h"
 #include "commands.h"
 
-int lxc_make_abstract_socket_name(char *path, int len, const char *lxcname,
+int lxc_make_abstract_socket_name(char *path, size_t pathlen,
+				  const char *lxcname,
 				  const char *lxcpath,
 				  const char *hashed_sock_name,
 				  const char *suffix);


More information about the lxc-devel mailing list