From: Andre Noll <maan@tuebingen.mpg.de>
Date: Sat, 27 Apr 2024 21:45:59 +0000 (+0200)
Subject: Resolve config file path only once.
X-Git-Tag: v1.0.2~29
X-Git-Url: http://git.tue.mpg.de/?a=commitdiff_plain;h=73c6357dd381ec9b30d0db7aa1a0bc97ad942ea3;p=dss.git

Resolve config file path only once.

Currently we call get_config_file_name() before each call call to a
public function of ipc.c because those functions compute the IPC key
from the filename. This is unnecessary and caused several bugs. Clean
up this mess by computing the filename only once and store it in a
global variable.
---

diff --git a/dss.c b/dss.c
index 0992ec6..e709d34 100644
--- a/dss.c
+++ b/dss.c
@@ -65,6 +65,8 @@ static const struct lls_command *subcmd;
 static bool daemonized;
 /** Non-NULL if we log to a file. */
 static FILE *logfile;
+/* Realpath of the config file. */
+static char *config_file;
 /** The read end of the signal pipe */
 static int signal_pipe;
 /** Process id of current pre-create-hook/rsync/post-create-hook process. */
@@ -271,27 +273,37 @@ static __printf_1_2 void dss_msg(const char* fmt,...)
 	va_end(argp);
 }
 
-static char *get_config_file_name(void)
+static void set_config_file_name(void)
 {
-	char *home, *config_file;
 
-	if (OPT_GIVEN(DSS, CONFIG_FILE))
-		return dss_strdup(OPT_STRING_VAL(DSS, CONFIG_FILE));
-	home = get_homedir();
-	config_file = make_message("%s/.dssrc", home);
-	free(home);
-	return config_file;
+	if (OPT_GIVEN(DSS, CONFIG_FILE)) {
+		const char *arg = OPT_STRING_VAL(DSS, CONFIG_FILE);
+		config_file = realpath(arg, NULL);
+		if (!config_file) {
+			DSS_EMERG_LOG(("could not resolve path %s: %s\n", arg,
+				strerror(errno)));
+			exit(EXIT_FAILURE);
+		}
+	} else {
+		char *home = get_homedir();
+		char *arg = make_message("%s/.dssrc", home);
+		free(home);
+		config_file = realpath(arg, NULL);
+		if (config_file)
+			free(arg);
+		else /* not fatal */
+			config_file = arg;
+	}
+	DSS_DEBUG_LOG(("config file: %s\n", config_file));
 }
 
 static int send_signal(int sig, bool wait)
 {
 	pid_t pid;
-	char *config_file = get_config_file_name();
 	int ret = get_dss_pid(config_file, &pid);
 	unsigned ms = 32;
 	struct timespec ts;
 
-	free(config_file);
 	if (ret < 0)
 		return ret;
 	if (OPT_GIVEN(DSS, DRY_RUN)) {
@@ -1166,7 +1178,6 @@ static int lopsub_error(int lopsub_ret, char **errctx)
 static int parse_config_file(bool sighup, const struct lls_command *cmd)
 {
 	int ret, fd = -1;
-	char *config_file = get_config_file_name();
 	struct stat statbuf;
 	void *map;
 	size_t sz;
@@ -1258,7 +1269,6 @@ close_fd:
 	if (fd >= 0)
 		close(fd);
 out:
-	free(config_file);
 	return ret;
 }
 
@@ -1589,10 +1599,8 @@ static void exit_hook(int exit_code)
 
 static void lock_dss_or_die(void)
 {
-	char *config_file = get_config_file_name();
 	int ret = lock_dss(config_file);
 
-	free(config_file);
 	if (ret < 0) {
 		DSS_EMERG_LOG(("failed to lock: %s\n", dss_strerror(-ret)));
 		exit(EXIT_FAILURE);
@@ -1602,16 +1610,13 @@ static void lock_dss_or_die(void)
 static int com_run(void)
 {
 	int ret, fd = -1;
-	char *config_file;
 	pid_t pid;
 
 	if (OPT_GIVEN(DSS, DRY_RUN)) {
 		DSS_ERROR_LOG(("dry run not supported by this command\n"));
 		return -E_SYNTAX;
 	}
-	config_file = get_config_file_name();
 	ret = get_dss_pid(config_file, &pid);
-	free(config_file);
 	if (ret >= 0) {
 		DSS_ERROR_LOG(("pid %d\n", (int)pid));
 		return -E_ALREADY_RUNNING;
@@ -1864,6 +1869,7 @@ int main(int argc, char **argv)
 		goto out;
 	}
 	lpr = cmdline_lpr;
+	set_config_file_name();
 	ret = parse_config_file(false /* no SIGHUP */, CMD_PTR(DSS));
 	if (ret < 0)
 		goto out;
@@ -1909,5 +1915,6 @@ out:
 	lls_free_parse_result(sublpr, subcmd);
 	if (sublpr != cmdline_sublpr)
 		lls_free_parse_result(cmdline_sublpr, subcmd);
+	free(config_file);
 	exit(ret >= 0? EXIT_SUCCESS : EXIT_FAILURE);
 }
diff --git a/ipc.c b/ipc.c
index 4aef302..2ccd156 100644
--- a/ipc.c
+++ b/ipc.c
@@ -80,31 +80,8 @@ static uint32_t super_fast_hash(const uint8_t *data, uint32_t len, uint32_t hash
 
 static int get_key_or_die(const char *config_file)
 {
-	int ret;
-	struct stat statbuf;
-	char *rpath;
-
 	assert(config_file);
-	if (stat(config_file, &statbuf) == 0) {
-		rpath = realpath(config_file, NULL);
-		if (!rpath) {
-			DSS_EMERG_LOG(("could not resolve path %s: %m\n", config_file));
-			exit(EXIT_FAILURE);
-		}
-		DSS_DEBUG_LOG(("resolved path: %s\n", rpath));
-	} else
-		/*
-		 * This happens if the user did not specify a config file, and
-		 * the default config file does not exist.  Another (unlikely)
-		 * possibility is that the config file was removed between
-		 * startup and this call. We don't care about these corner
-		 * cases too much and just use the unresolved path in this
-		 * case.
-		 */
-		rpath = dss_strdup(config_file);
-	ret = super_fast_hash((uint8_t *)rpath, strlen(rpath), 0) >> 1;
-	free(rpath);
-	return ret;
+	return super_fast_hash((uint8_t *)config_file, strlen(config_file), 0) >> 1;
 }
 
 static int mutex_get(key_t key, int flags)