[PATCH 3/8] unix-stream-server: create unix-stream-server.c

["Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com>]

From: Jeff Hostetler <jeffhost@microsoft.com>

Move unix_stream_server__listen_with_lock() and associated routines
and data types out of unix-socket.[ch] and into their own source file.

We added code to safely create a Unix domain socket using a lockfile in:
9406d12e14 (unix-socket: create `unix_stream_server__listen_with_lock()`,
2021-02-13).  This code conceptually exists at a higher-level than the
core unix_stream_connect() and unix_stream_listen() routines that it
consumes and should be isolated for clarity.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 Makefile                            |   1 +
 compat/simple-ipc/ipc-unix-socket.c |   1 +
 contrib/buildsystems/CMakeLists.txt |   2 +-
 unix-socket.c                       | 120 ---------------------------
 unix-socket.h                       |  26 ------
 unix-stream-server.c                | 124 ++++++++++++++++++++++++++++
 unix-stream-server.h                |  32 +++++++
 7 files changed, 159 insertions(+), 147 deletions(-)
 create mode 100644 unix-stream-server.c
 create mode 100644 unix-stream-server.h

diff --git a/Makefile b/Makefile
index 93f2e7ca9e1f..dfc37b0480e4 100644
--- a/Makefile
+++ b/Makefile
@@ -1678,6 +1678,7 @@ ifdef NO_UNIX_SOCKETS
 	BASIC_CFLAGS += -DNO_UNIX_SOCKETS
 else
 	LIB_OBJS += unix-socket.o
+	LIB_OBJS += unix-stream-server.o
 	LIB_OBJS += compat/simple-ipc/ipc-shared.o
 	LIB_OBJS += compat/simple-ipc/ipc-unix-socket.o
 endif
diff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c
index b7fd0b34329e..b0264ca6cdc6 100644
--- a/compat/simple-ipc/ipc-unix-socket.c
+++ b/compat/simple-ipc/ipc-unix-socket.c
@@ -4,6 +4,7 @@
 #include "pkt-line.h"
 #include "thread-utils.h"
 #include "unix-socket.h"
+#include "unix-stream-server.h"
 
 #ifdef NO_UNIX_SOCKETS
 #error compat/simple-ipc/ipc-unix-socket.c requires Unix sockets
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 4c27a373414a..629a1817459b 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -243,7 +243,7 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
 
 elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux")
 	add_compile_definitions(PROCFS_EXECUTABLE_PATH="/proc/self/exe" HAVE_DEV_TTY )
-	list(APPEND compat_SOURCES unix-socket.c)
+	list(APPEND compat_SOURCES unix-socket.c unix-stream-server.c)
 endif()
 
 if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
diff --git a/unix-socket.c b/unix-socket.c
index c9ea1de43bd2..e0be1badb58d 100644
--- a/unix-socket.c
+++ b/unix-socket.c
@@ -1,8 +1,6 @@
 #include "cache.h"
-#include "lockfile.h"
 #include "unix-socket.h"
 
-#define DEFAULT_UNIX_STREAM_LISTEN_TIMEOUT (100)
 #define DEFAULT_UNIX_STREAM_LISTEN_BACKLOG (5)
 
 static int chdir_len(const char *orig, int len)
@@ -136,121 +134,3 @@ int unix_stream_listen(const char *path,
 	errno = saved_errno;
 	return -1;
 }
-
-static int is_another_server_alive(const char *path,
-				   const struct unix_stream_listen_opts *opts)
-{
-	struct stat st;
-	int fd;
-
-	if (!lstat(path, &st) && S_ISSOCK(st.st_mode)) {
-		/*
-		 * A socket-inode exists on disk at `path`, but we
-		 * don't know whether it belongs to an active server
-		 * or whether the last server died without cleaning
-		 * up.
-		 *
-		 * Poke it with a trivial connection to try to find
-		 * out.
-		 */
-		fd = unix_stream_connect(path, opts->disallow_chdir);
-		if (fd >= 0) {
-			close(fd);
-			return 1;
-		}
-	}
-
-	return 0;
-}
-
-struct unix_stream_server_socket *unix_stream_server__listen_with_lock(
-	const char *path,
-	const struct unix_stream_listen_opts *opts)
-{
-	struct lock_file lock = LOCK_INIT;
-	long timeout;
-	int fd_socket;
-	struct unix_stream_server_socket *server_socket;
-
-	timeout = opts->timeout_ms;
-	if (opts->timeout_ms <= 0)
-		timeout = DEFAULT_UNIX_STREAM_LISTEN_TIMEOUT;
-
-	/*
-	 * Create a lock at "<path>.lock" if we can.
-	 */
-	if (hold_lock_file_for_update_timeout(&lock, path, 0, timeout) < 0) {
-		error_errno(_("could not lock listener socket '%s'"), path);
-		return NULL;
-	}
-
-	/*
-	 * If another server is listening on "<path>" give up.  We do not
-	 * want to create a socket and steal future connections from them.
-	 */
-	if (is_another_server_alive(path, opts)) {
-		errno = EADDRINUSE;
-		error_errno(_("listener socket already in use '%s'"), path);
-		rollback_lock_file(&lock);
-		return NULL;
-	}
-
-	/*
-	 * Create and bind to a Unix domain socket at "<path>".
-	 */
-	fd_socket = unix_stream_listen(path, opts);
-	if (fd_socket < 0) {
-		error_errno(_("could not create listener socket '%s'"), path);
-		rollback_lock_file(&lock);
-		return NULL;
-	}
-
-	server_socket = xcalloc(1, sizeof(*server_socket));
-	server_socket->path_socket = strdup(path);
-	server_socket->fd_socket = fd_socket;
-	lstat(path, &server_socket->st_socket);
-
-	/*
-	 * Always rollback (just delete) "<path>.lock" because we already created
-	 * "<path>" as a socket and do not want to commit_lock to do the atomic
-	 * rename trick.
-	 */
-	rollback_lock_file(&lock);
-
-	return server_socket;
-}
-
-void unix_stream_server__free(
-	struct unix_stream_server_socket *server_socket)
-{
-	if (!server_socket)
-		return;
-
-	if (server_socket->fd_socket >= 0) {
-		if (!unix_stream_server__was_stolen(server_socket))
-			unlink(server_socket->path_socket);
-		close(server_socket->fd_socket);
-	}
-
-	free(server_socket->path_socket);
-	free(server_socket);
-}
-
-int unix_stream_server__was_stolen(
-	struct unix_stream_server_socket *server_socket)
-{
-	struct stat st_now;
-
-	if (!server_socket)
-		return 0;
-
-	if (lstat(server_socket->path_socket, &st_now) == -1)
-		return 1;
-
-	if (st_now.st_ino != server_socket->st_socket.st_ino)
-		return 1;
-
-	/* We might also consider the ctime on some platforms. */
-
-	return 0;
-}
diff --git a/unix-socket.h b/unix-socket.h
index bec925ee0213..b079e79cb3e1 100644
--- a/unix-socket.h
+++ b/unix-socket.h
@@ -18,30 +18,4 @@ int unix_stream_connect(const char *path, int disallow_chdir);
 int unix_stream_listen(const char *path,
 		       const struct unix_stream_listen_opts *opts);
 
-struct unix_stream_server_socket {
-	char *path_socket;
-	struct stat st_socket;
-	int fd_socket;
-};
-
-/*
- * Create a Unix Domain Socket at the given path under the protection
- * of a '.lock' lockfile.
- */
-struct unix_stream_server_socket *unix_stream_server__listen_with_lock(
-	const char *path,
-	const struct unix_stream_listen_opts *opts);
-
-/*
- * Close and delete the socket.
- */
-void unix_stream_server__free(
-	struct unix_stream_server_socket *server_socket);
-
-/*
- * Return 1 if the inode of the pathname to our socket changes.
- */
-int unix_stream_server__was_stolen(
-	struct unix_stream_server_socket *server_socket);
-
 #endif /* UNIX_SOCKET_H */
diff --git a/unix-stream-server.c b/unix-stream-server.c
new file mode 100644
index 000000000000..ad175ba731ca
--- /dev/null
+++ b/unix-stream-server.c
@@ -0,0 +1,124 @@
+#include "cache.h"
+#include "lockfile.h"
+#include "unix-socket.h"
+#include "unix-stream-server.h"
+
+#define DEFAULT_UNIX_STREAM_LISTEN_TIMEOUT (100)
+
+static int is_another_server_alive(const char *path,
+				   const struct unix_stream_listen_opts *opts)
+{
+	struct stat st;
+	int fd;
+
+	if (!lstat(path, &st) && S_ISSOCK(st.st_mode)) {
+		/*
+		 * A socket-inode exists on disk at `path`, but we
+		 * don't know whether it belongs to an active server
+		 * or whether the last server died without cleaning
+		 * up.
+		 *
+		 * Poke it with a trivial connection to try to find
+		 * out.
+		 */
+		fd = unix_stream_connect(path, opts->disallow_chdir);
+		if (fd >= 0) {
+			close(fd);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+struct unix_stream_server_socket *unix_stream_server__listen_with_lock(
+	const char *path,
+	const struct unix_stream_listen_opts *opts)
+{
+	struct lock_file lock = LOCK_INIT;
+	long timeout;
+	int fd_socket;
+	struct unix_stream_server_socket *server_socket;
+
+	timeout = opts->timeout_ms;
+	if (opts->timeout_ms <= 0)
+		timeout = DEFAULT_UNIX_STREAM_LISTEN_TIMEOUT;
+
+	/*
+	 * Create a lock at "<path>.lock" if we can.
+	 */
+	if (hold_lock_file_for_update_timeout(&lock, path, 0, timeout) < 0) {
+		error_errno(_("could not lock listener socket '%s'"), path);
+		return NULL;
+	}
+
+	/*
+	 * If another server is listening on "<path>" give up.  We do not
+	 * want to create a socket and steal future connections from them.
+	 */
+	if (is_another_server_alive(path, opts)) {
+		errno = EADDRINUSE;
+		error_errno(_("listener socket already in use '%s'"), path);
+		rollback_lock_file(&lock);
+		return NULL;
+	}
+
+	/*
+	 * Create and bind to a Unix domain socket at "<path>".
+	 */
+	fd_socket = unix_stream_listen(path, opts);
+	if (fd_socket < 0) {
+		error_errno(_("could not create listener socket '%s'"), path);
+		rollback_lock_file(&lock);
+		return NULL;
+	}
+
+	server_socket = xcalloc(1, sizeof(*server_socket));
+	server_socket->path_socket = strdup(path);
+	server_socket->fd_socket = fd_socket;
+	lstat(path, &server_socket->st_socket);
+
+	/*
+	 * Always rollback (just delete) "<path>.lock" because we already created
+	 * "<path>" as a socket and do not want to commit_lock to do the atomic
+	 * rename trick.
+	 */
+	rollback_lock_file(&lock);
+
+	return server_socket;
+}
+
+void unix_stream_server__free(
+	struct unix_stream_server_socket *server_socket)
+{
+	if (!server_socket)
+		return;
+
+	if (server_socket->fd_socket >= 0) {
+		if (!unix_stream_server__was_stolen(server_socket))
+			unlink(server_socket->path_socket);
+		close(server_socket->fd_socket);
+	}
+
+	free(server_socket->path_socket);
+	free(server_socket);
+}
+
+int unix_stream_server__was_stolen(
+	struct unix_stream_server_socket *server_socket)
+{
+	struct stat st_now;
+
+	if (!server_socket)
+		return 0;
+
+	if (lstat(server_socket->path_socket, &st_now) == -1)
+		return 1;
+
+	if (st_now.st_ino != server_socket->st_socket.st_ino)
+		return 1;
+
+	/* We might also consider the ctime on some platforms. */
+
+	return 0;
+}
diff --git a/unix-stream-server.h b/unix-stream-server.h
new file mode 100644
index 000000000000..745849e31c30
--- /dev/null
+++ b/unix-stream-server.h
@@ -0,0 +1,32 @@
+#ifndef UNIX_STREAM_SERVER_H
+#define UNIX_STREAM_SERVER_H
+
+#include "unix-socket.h"
+
+struct unix_stream_server_socket {
+	char *path_socket;
+	struct stat st_socket;
+	int fd_socket;
+};
+
+/*
+ * Create a Unix Domain Socket at the given path under the protection
+ * of a '.lock' lockfile.
+ */
+struct unix_stream_server_socket *unix_stream_server__listen_with_lock(
+	const char *path,
+	const struct unix_stream_listen_opts *opts);
+
+/*
+ * Close and delete the socket.
+ */
+void unix_stream_server__free(
+	struct unix_stream_server_socket *server_socket);
+
+/*
+ * Return 1 if the inode of the pathname to our socket changes.
+ */
+int unix_stream_server__was_stolen(
+	struct unix_stream_server_socket *server_socket);
+
+#endif /* UNIX_STREAM_SERVER_H */
-- 
gitgitgadget