lightningd: Later --alt-subdaemon args replace earlier. Code style im…

…provements.

doc/lightningd-config.5

@@ -126,8 +126,7 @@ by \fI--conf\fR\.
  \fBalt-subdaemon\fR=\fISUBDAEMON\fR:\fIPATH\fR
 Specifies an alternate subdaemon binary\.  If the supplied path
 is relative the subdaemon binary is found in the working directory\.
-This option may be specified multiple times, but only once for
-each subdaemon\.
+This option may be specified multiple times\.
 
 
  So, \fBalt-subdaemon=lightning_hsmd:remote_signer\fR would use a

doc/lightningd-config.5.md

@@ -110,8 +110,7 @@ by *--conf*.
  **alt-subdaemon**=*SUBDAEMON*:*PATH*
 Specifies an alternate subdaemon binary.  If the supplied path
 is relative the subdaemon binary is found in the working directory.
-This option may be specified multiple times, but only once for
-each subdaemon.
+This option may be specified multiple times.
 
  So, **alt-subdaemon=lightning_hsmd:remote_signer** would use a
 hypothetical remote signing proxy instead of the standard *lightning_hsmd*

lightningd/lightningd.c

@@ -298,31 +298,17 @@ static void memleak_help_alt_subdaemons(struct htable *memtable,
 }
 #endif /* DEVELOPER */
 
-const char *subdaemon_path(const struct lightningd *ld, const char *name)
+const char *subdaemon_path(const tal_t *ctx, const struct lightningd *ld, const char *name)
 {
-	const char *dpath;
-
-	/*~ CCAN's path module uses tal, so wants a context to
-	 * allocate from.  We have a magic convenience context
-	 * `tmpctx` for temporary allocations like this.
-	 *
-	 * Because all our daemons at their core are of form `while
-	 * (!stopped) handle_events();` (an event loop pattern), we
-	 * can free `tmpctx` in that top-level loop after each event
-	 * is handled.
-	 */
-
 	/* Is there an alternate path for this subdaemon? */
+	const char *dpath;
 	const char *alt = strmap_get(&ld->alt_subdaemons, name);
 	if (alt) {
-		/* Is the alternate path absolute? */
-		if (path_is_abs(alt))
-			dpath = tal_strdup(tmpctx, alt);
-		else
-			dpath = path_join(tmpctx, ld->daemon_dir, alt);
+		/* path_join will honor absolute paths as well. */
+		dpath = path_join(ctx, ld->daemon_dir, alt);
 	} else {
 		/* This subdaemon is found in the standard place. */
-		dpath = path_join(tmpctx, ld->daemon_dir, name);
+		dpath = path_join(ctx, ld->daemon_dir, name);
 	}
 	return dpath;
 }
@@ -341,8 +327,17 @@ void test_subdaemons(const struct lightningd *ld)
 	 * ARRAY_SIZE will cause a compiler error if the argument is actually
 	 * a pointer, not an array. */
 	for (i = 0; i < ARRAY_SIZE(subdaemons); i++) {
+		/*~ CCAN's path module uses tal, so wants a context to
+		 * allocate from.  We have a magic convenience context
+		 * `tmpctx` for temporary allocations like this.
+		 *
+		 * Because all our daemons at their core are of form `while
+		 * (!stopped) handle_events();` (an event loop pattern), we
+		 * can free `tmpctx` in that top-level loop after each event
+		 * is handled.
+		 */
 		int outfd;
-		const char *dpath = subdaemon_path(ld, subdaemons[i]);
+		const char *dpath = subdaemon_path(tmpctx, ld, subdaemons[i]);
 		const char *verstring;
 		/*~ CCAN's pipecmd module is like popen for grownups: it
 		 * takes pointers to fill in stdin, stdout and stderr file

lightningd/lightningd.h

@@ -262,7 +262,7 @@ extern bool tal_oom_ok;
 bool is_subdaemon(const char *sdname);
 
 /* Returns the path to the subdaemon. Considers alternate subdaemon paths. */
-const char *subdaemon_path(const struct lightningd *ld, const char *name);
+const char *subdaemon_path(const tal_t *ctx, const struct lightningd *ld, const char *name);
 
 /* Check we can run subdaemons, and check their versions */
 void test_subdaemons(const struct lightningd *ld);

lightningd/options.c

@@ -193,6 +193,11 @@ static char *opt_add_addr(const char *arg, struct lightningd *ld)
 
 static char *opt_alt_subdaemon(const char *arg, struct lightningd *ld)
 {
+	char *subdaemon;
+	char *sdpath;
+	char *prevname;
+	char *prevval;
+
 	/* example arg: "lightning_hsmd:remote_hsmd" */
 	char *argbuf = tal_strdup(ld, arg);
 	char *colon = strchr(argbuf, ':');
@@ -203,10 +208,17 @@ static char *opt_alt_subdaemon(const char *arg, struct lightningd *ld)
 	if (!is_subdaemon(argbuf))
 		return tal_fmt(NULL, "%s is not a subdaemon", argbuf);
 
-	char *subdaemon = tal_strdup(ld, argbuf);
-	char *sdpath = tal_strdup(ld, colon+1);
-	if (!strmap_add(&ld->alt_subdaemons, subdaemon, sdpath))
-		return tal_fmt(NULL, "%s already exists", argbuf);
+	subdaemon = tal_strdup(ld, argbuf);
+
+	/* Remove any preexisting alt subdaemon mapping. */
+	prevname = strmap_del(&ld->alt_subdaemons, subdaemon, (const char **) &prevval);
+	if (prevname) {
+		tal_free(prevname);
+		tal_free(prevval);
+	}
+
+	sdpath = tal_strdup(ld, colon+1);
+	strmap_add(&ld->alt_subdaemons, subdaemon, sdpath);
 
 	tal_free(argbuf);
 	return NULL;
@@ -868,8 +880,8 @@ static void register_opts(struct lightningd *ld)
 			 "Specifies an alternate subdaemon binary. "
 			 "If the supplied path is relative the subdaemon "
 			 "binary is found in the working directory. "
-			 "This option may be specified multiple times, "
-			 "but only once for each subdaemon. For example, "
+			 "This option may be specified multiple times. "
+			 "For example, "
 			 "--alt-subdaemon=lightning_hsmd:remote_signer "
 			 "would use a hypothetical remote signing subdaemon.");
 

lightningd/subd.c

@@ -648,7 +648,7 @@ static struct subd *new_subd(struct lightningd *ld,
 	disconnect_fd = ld->dev_disconnect_fd;
 #endif /* DEVELOPER */
 
-	const char *path = subdaemon_path(ld, name);
+	const char *path = subdaemon_path(tmpctx, ld, name);
 
 	sd->pid = subd(path, name, debug_subd,
 		       &msg_fd, disconnect_fd,