Merge branch 'rs/strbuf-expand'

* rs/strbuf-expand:
  remove the unused files interpolate.c and interpolate.h
  daemon: deglobalize variable 'directory'
  daemon: inline fill_in_extra_table_entries()
  daemon: use strbuf_expand() instead of interpolate()
  merge-recursive: use strbuf_expand() instead of interpolate()
  add strbuf_expand_dict_cb(), a helper for simple cases

Documentation/technical/api-strbuf.txt

@@ -205,6 +205,13 @@ In order to facilitate caching and to make it possible to give
 parameters to the callback, `strbuf_expand()` passes a context pointer,
 which can be used by the programmer of the callback as she sees fit.
 
+`strbuf_expand_dict_cb`::
+
+	Used as callback for `strbuf_expand()`, expects an array of
+	struct strbuf_expand_dict_entry as context, i.e. pairs of
+	placeholder and replacement string.  The array needs to be
+	terminated by an entry with placeholder set to NULL.
+
 `strbuf_addf`::
 
 	Add a formatted string to the buffer.

Makefile

@@ -437,7 +437,6 @@ LIB_OBJS += grep.o
 LIB_OBJS += hash.o
 LIB_OBJS += help.o
 LIB_OBJS += ident.o
-LIB_OBJS += interpolate.o
 LIB_OBJS += levenshtein.o
 LIB_OBJS += list-objects.o
 LIB_OBJS += ll-merge.o

daemon.c

@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "pkt-line.h"
 #include "exec_cmd.h"
-#include "interpolate.h"
 
 #include <syslog.h>
 
@@ -54,26 +53,10 @@ static const char *user_path;
 static unsigned int timeout;
 static unsigned int init_timeout;
 
-/*
+static char *hostname;
- * Static table for now.  Ugh.
+static char *canon_hostname;
- * Feel free to make dynamic as needed.
+static char *ip_address;
- */
+static char *tcp_port;
-#define INTERP_SLOT_HOST	(0)
-#define INTERP_SLOT_CANON_HOST	(1)
-#define INTERP_SLOT_IP		(2)
-#define INTERP_SLOT_PORT	(3)
-#define INTERP_SLOT_DIR		(4)
-#define INTERP_SLOT_PERCENT	(5)
-
-static struct interp interp_table[] = {
-	{ "%H", 0},
-	{ "%CH", 0},
-	{ "%IP", 0},
-	{ "%P", 0},
-	{ "%D", 0},
-	{ "%%", 0},
-};
-
 
 static void logreport(int priority, const char *err, va_list params)
 {
@@ -163,7 +146,7 @@ static int avoid_alias(char *p)
 	}
 }
 
-static char *path_ok(struct interp *itable)
+static char *path_ok(char *directory)
 {
 	static char rpath[PATH_MAX];
 	static char interp_path[PATH_MAX];
@@ -171,7 +154,7 @@ static char *path_ok(struct interp *itable)
 	char *path;
 	char *dir;
 
-	dir = itable[INTERP_SLOT_DIR].value;
+	dir = directory;
 
 	if (avoid_alias(dir)) {
 		logerror("'%s': aliased", dir);
@@ -201,14 +184,27 @@ static char *path_ok(struct interp *itable)
 		}
 	}
 	else if (interpolated_path && saw_extended_args) {
+		struct strbuf expanded_path = STRBUF_INIT;
+		struct strbuf_expand_dict_entry dict[] = {
+			{ "H", hostname },
+			{ "CH", canon_hostname },
+			{ "IP", ip_address },
+			{ "P", tcp_port },
+			{ "D", directory },
+			{ "%", "%" },
+			{ NULL }
+		};
+
 		if (*dir != '/') {
 			/* Allow only absolute */
 			logerror("'%s': Non-absolute path denied (interpolated-path active)", dir);
 			return NULL;
 		}
 
-		interpolate(interp_path, PATH_MAX, interpolated_path,
+		strbuf_expand(&expanded_path, interpolated_path,
-			    interp_table, ARRAY_SIZE(interp_table));
+				strbuf_expand_dict_cb, &dict);
+		strlcpy(interp_path, expanded_path.buf, PATH_MAX);
+		strbuf_release(&expanded_path);
 		loginfo("Interpolated dir '%s'", interp_path);
 
 		dir = interp_path;
@@ -233,7 +229,7 @@ static char *path_ok(struct interp *itable)
 		 * prefixing the base path
 		 */
 		if (base_path && base_path_relaxed && !retried_path) {
-			dir = itable[INTERP_SLOT_DIR].value;
+			dir = directory;
 			retried_path = 1;
 			continue;
 		}
@@ -299,14 +295,12 @@ static int git_daemon_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
-static int run_service(struct interp *itable, struct daemon_service *service)
+static int run_service(char *dir, struct daemon_service *service)
 {
 	const char *path;
 	int enabled = service->enabled;
 
-	loginfo("Request %s for '%s'",
+	loginfo("Request %s for '%s'", service->name, dir);
-		service->name,
-		itable[INTERP_SLOT_DIR].value);
 
 	if (!enabled && !service->overridable) {
 		logerror("'%s': service not enabled.", service->name);
@@ -314,7 +308,7 @@ static int run_service(struct interp *itable, struct daemon_service *service)
 		return -1;
 	}
 
-	if (!(path = path_ok(itable)))
+	if (!(path = path_ok(dir)))
 		return -1;
 
 	/*
@@ -413,13 +407,13 @@ static void make_service_overridable(const char *name, int ena)
 
 /*
  * Separate the "extra args" information as supplied by the client connection.
- * Any resulting data is squirreled away in the given interpolation table.
  */
-static void parse_extra_args(struct interp *table, char *extra_args, int buflen)
+static void parse_extra_args(char *extra_args, int buflen)
 {
 	char *val;
 	int vallen;
 	char *end = extra_args + buflen;
+	char *hp;
 
 	while (extra_args < end && *extra_args) {
 		saw_extended_args = 1;
@@ -433,25 +427,22 @@ static void parse_extra_args(struct interp *table, char *extra_args, int buflen)
 				if (port) {
 					*port = 0;
 					port++;
-					interp_set_entry(table, INTERP_SLOT_PORT, port);
+					free(tcp_port);
+					tcp_port = xstrdup(port);
 				}
-				interp_set_entry(table, INTERP_SLOT_HOST, host);
+				free(hostname);
+				hostname = xstrdup(host);
 			}
 
 			/* On to the next one */
 			extra_args = val + vallen;
 		}
 	}
-}
-
-static void fill_in_extra_table_entries(struct interp *itable)
-{
-	char *hp;
 
 	/*
 	 * Replace literal host with lowercase-ized hostname.
 	 */
-	hp = interp_table[INTERP_SLOT_HOST].value;
+	hp = hostname;
 	if (!hp)
 		return;
 	for ( ; *hp; hp++)
@@ -470,17 +461,17 @@ static void fill_in_extra_table_entries(struct interp *itable)
 		memset(&hints, 0, sizeof(hints));
 		hints.ai_flags = AI_CANONNAME;
 
-		gai = getaddrinfo(interp_table[INTERP_SLOT_HOST].value, 0, &hints, &ai0);
+		gai = getaddrinfo(hostname, 0, &hints, &ai0);
 		if (!gai) {
 			for (ai = ai0; ai; ai = ai->ai_next) {
 				struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
 
 				inet_ntop(AF_INET, &sin_addr->sin_addr,
 					  addrbuf, sizeof(addrbuf));
-				interp_set_entry(interp_table,
+				free(canon_hostname);
-						 INTERP_SLOT_CANON_HOST, ai->ai_canonname);
+				canon_hostname = xstrdup(ai->ai_canonname);
-				interp_set_entry(interp_table,
+				free(ip_address);
-						 INTERP_SLOT_IP, addrbuf);
+				ip_address = xstrdup(addrbuf);
 				break;
 			}
 			freeaddrinfo(ai0);
@@ -493,7 +484,7 @@ static void fill_in_extra_table_entries(struct interp *itable)
 		char **ap;
 		static char addrbuf[HOST_NAME_MAX + 1];
 
-		hent = gethostbyname(interp_table[INTERP_SLOT_HOST].value);
+		hent = gethostbyname(hostname);
 
 		ap = hent->h_addr_list;
 		memset(&sa, 0, sizeof sa);
@@ -504,8 +495,10 @@ static void fill_in_extra_table_entries(struct interp *itable)
 		inet_ntop(hent->h_addrtype, &sa.sin_addr,
 			  addrbuf, sizeof(addrbuf));
 
-		interp_set_entry(interp_table, INTERP_SLOT_CANON_HOST, hent->h_name);
+		free(canon_hostname);
-		interp_set_entry(interp_table, INTERP_SLOT_IP, addrbuf);
+		canon_hostname = xstrdup(hent->h_name);
+		free(ip_address);
+		ip_address = xstrdup(addrbuf);
 	}
 #endif
 }
@@ -557,16 +550,14 @@ static int execute(struct sockaddr *addr)
 		pktlen--;
 	}
 
-	/*
+	free(hostname);
-	 * Initialize the path interpolation table for this connection.
+	free(canon_hostname);
-	 */
+	free(ip_address);
-	interp_clear_table(interp_table, ARRAY_SIZE(interp_table));
+	free(tcp_port);
-	interp_set_entry(interp_table, INTERP_SLOT_PERCENT, "%");
+	hostname = canon_hostname = ip_address = tcp_port = NULL;
 
-	if (len != pktlen) {
+	if (len != pktlen)
-	    parse_extra_args(interp_table, line + len + 1, pktlen - len - 1);
+		parse_extra_args(line + len + 1, pktlen - len - 1);
-	    fill_in_extra_table_entries(interp_table);
-	}
 
 	for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
 		struct daemon_service *s = &(daemon_service[i]);
@@ -578,9 +569,7 @@ static int execute(struct sockaddr *addr)
 			 * Note: The directory here is probably context sensitive,
 			 * and might depend on the actual service being performed.
 			 */
-			interp_set_entry(interp_table,
+			return run_service(line + namelen + 5, s);
-					 INTERP_SLOT_DIR, line + namelen + 5);
-			return run_service(interp_table, s);
 		}
 	}
 

interpolate.c

@@ -1,103 +0,0 @@
-/*
- * Copyright 2006 Jon Loeliger
- */
-
-#include "git-compat-util.h"
-#include "interpolate.h"
-
-
-void interp_set_entry(struct interp *table, int slot, const char *value)
-{
-	char *oldval = table[slot].value;
-	char *newval = NULL;
-
-	free(oldval);
-
-	if (value)
-		newval = xstrdup(value);
-
-	table[slot].value = newval;
-}
-
-
-void interp_clear_table(struct interp *table, int ninterps)
-{
-	int i;
-
-	for (i = 0; i < ninterps; i++) {
-		interp_set_entry(table, i, NULL);
-	}
-}
-
-
-/*
- * Convert a NUL-terminated string in buffer orig
- * into the supplied buffer, result, whose length is reslen,
- * performing substitutions on %-named sub-strings from
- * the table, interps, with ninterps entries.
- *
- * Example interps:
- *    {
- *        { "%H", "example.org"},
- *        { "%port", "123"},
- *        { "%%", "%"},
- *    }
- *
- * Returns the length of the substituted string (not including the final \0).
- * Like with snprintf, if the result is >= reslen, then it overflowed.
- */
-
-unsigned long interpolate(char *result, unsigned long reslen,
-		const char *orig,
-		const struct interp *interps, int ninterps)
-{
-	const char *src = orig;
-	char *dest = result;
-	unsigned long newlen = 0;
-	const char *name, *value;
-	unsigned long namelen, valuelen;
-	int i;
-	char c;
-
-	while ((c = *src)) {
-		if (c == '%') {
-			/* Try to match an interpolation string. */
-			for (i = 0; i < ninterps; i++) {
-				name = interps[i].name;
-				namelen = strlen(name);
-				if (strncmp(src, name, namelen) == 0)
-					break;
-			}
-
-			/* Check for valid interpolation. */
-			if (i < ninterps) {
-				value = interps[i].value;
-				if (!value) {
-					src += namelen;
-					continue;
-				}
-
-				valuelen = strlen(value);
-				if (newlen + valuelen < reslen) {
-					/* Substitute. */
-					memcpy(dest, value, valuelen);
-					dest += valuelen;
-				}
-				newlen += valuelen;
-				src += namelen;
-				continue;
-			}
-		}
-		/* Straight copy one non-interpolation character. */
-		if (newlen + 1 < reslen)
-			*dest++ = *src;
-		src++;
-		newlen++;
-	}
-
-	/* XXX: the previous loop always keep room for the ending NUL,
-	   we just need to check if there was room for a NUL in the first place */
-	if (reslen > 0)
-		*dest = '\0';
-	return newlen;
-}

interpolate.h

@@ -1,26 +0,0 @@
-/*
- * Copyright 2006 Jon Loeliger
- */
-
-#ifndef INTERPOLATE_H
-#define INTERPOLATE_H
-
-/*
- * Convert a NUL-terminated string in buffer orig,
- * performing substitutions on %-named sub-strings from
- * the interpretation table.
- */
-
-struct interp {
-	const char *name;
-	char *value;
-};
-
-extern void interp_set_entry(struct interp *table, int slot, const char *value);
-extern void interp_clear_table(struct interp *table, int ninterps);
-
-extern unsigned long interpolate(char *result, unsigned long reslen,
-				 const char *orig,
-				 const struct interp *interps, int ninterps);
-
-#endif /* INTERPOLATE_H */

ll-merge.c

@@ -8,7 +8,6 @@
 #include "attr.h"
 #include "xdiff-interface.h"
 #include "run-command.h"
-#include "interpolate.h"
 #include "ll-merge.h"
 
 struct ll_merge_driver;
@@ -169,11 +168,12 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
 			int virtual_ancestor)
 {
 	char temp[3][50];
-	char cmdbuf[2048];
+	struct strbuf cmd = STRBUF_INIT;
-	struct interp table[] = {
+	struct strbuf_expand_dict_entry dict[] = {
-		{ "%O" },
+		{ "O", temp[0] },
-		{ "%A" },
+		{ "A", temp[1] },
-		{ "%B" },
+		{ "B", temp[2] },
+		{ NULL }
 	};
 	struct child_process child;
 	const char *args[20];
@@ -189,17 +189,13 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
 	create_temp(src1, temp[1]);
 	create_temp(src2, temp[2]);
 
-	interp_set_entry(table, 0, temp[0]);
+	strbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict);
-	interp_set_entry(table, 1, temp[1]);
-	interp_set_entry(table, 2, temp[2]);
-
-	interpolate(cmdbuf, sizeof(cmdbuf), fn->cmdline, table, 3);
 
 	memset(&child, 0, sizeof(child));
 	child.argv = args;
 	args[0] = "sh";
 	args[1] = "-c";
-	args[2] = cmdbuf;
+	args[2] = cmd.buf;
 	args[3] = NULL;
 
 	status = run_command(&child);
@@ -224,6 +220,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
  bad:
 	for (i = 0; i < 3; i++)
 		unlink(temp[i]);
+	strbuf_release(&cmd);
 	return status;
 }
 

merge-recursive.c

@@ -16,7 +16,6 @@
 #include "string-list.h"
 #include "xdiff-interface.h"
 #include "ll-merge.h"
-#include "interpolate.h"
 #include "attr.h"
 #include "merge-recursive.h"
 #include "dir.h"

strbuf.c

@@ -237,6 +237,22 @@ void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,
 	}
 }
 
+size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
+		void *context)
+{
+	struct strbuf_expand_dict_entry *e = context;
+	size_t len;
+
+	for (; e->placeholder && (len = strlen(e->placeholder)); e++) {
+		if (!strncmp(placeholder, e->placeholder, len)) {
+			if (e->value)
+				strbuf_addstr(sb, e->value);
+			return len;
+		}
+	}
+	return 0;
+}
+
 size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
 {
 	size_t res;

strbuf.h

@@ -111,6 +111,11 @@ extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);
 
 typedef size_t (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);
 extern void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn, void *context);
+struct strbuf_expand_dict_entry {
+	const char *placeholder;
+	const char *value;
+};
+extern size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder, void *context);
 
 __attribute__((format(printf,2,3)))
 extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);