From 9b864e730bda0e028a85387fa568429724582f22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 23 Nov 2008 00:09:30 +0100 Subject: [PATCH 1/6] add strbuf_expand_dict_cb(), a helper for simple cases The new callback function strbuf_expand_dict_cb() can be used together with strbuf_expand() if there is only a small number of placeholders for static replacement texts. It expects its dictionary as an array of placeholder+value pairs as context parameter, terminated by an entry with the placeholder member set to NULL. The new helper is intended to aid converting the remaining calls of interpolate(). strbuf_expand() is smaller, more flexible and can be used to go faster than interpolate(), so it should replace the latter. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- Documentation/technical/api-strbuf.txt | 7 +++++++ strbuf.c | 16 ++++++++++++++++ strbuf.h | 5 +++++ 3 files changed, 28 insertions(+) diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt index a9668e5f2..a8ee2fe6a 100644 --- a/Documentation/technical/api-strbuf.txt +++ b/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. diff --git a/strbuf.c b/strbuf.c index 720737d85..13be67e4d 100644 --- a/strbuf.c +++ b/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; diff --git a/strbuf.h b/strbuf.h index eba7ba423..b1670d994 100644 --- a/strbuf.h +++ b/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, ...); From ced621b2c15c3d3dd65dd1d7eec984f8546a4673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 23 Nov 2008 00:13:00 +0100 Subject: [PATCH 2/6] merge-recursive: use strbuf_expand() instead of interpolate() Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- ll-merge.c | 21 +++++++++------------ merge-recursive.c | 1 - 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/ll-merge.c b/ll-merge.c index 4a716146f..fa2ca5250 100644 --- a/ll-merge.c +++ b/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 interp table[] = { - { "%O" }, - { "%A" }, - { "%B" }, + struct strbuf cmd = STRBUF_INIT; + struct strbuf_expand_dict_entry dict[] = { + { "O", temp[0] }, + { "A", temp[1] }, + { "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]); - interp_set_entry(table, 1, temp[1]); - interp_set_entry(table, 2, temp[2]); - - interpolate(cmdbuf, sizeof(cmdbuf), fn->cmdline, table, 3); + strbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict); 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; } diff --git a/merge-recursive.c b/merge-recursive.c index 7472d3ecc..0e988f2a0 100644 --- a/merge-recursive.c +++ b/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" From 9d7ca667466b9e1a8160d20cce7c06d09213ab6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 23 Nov 2008 00:15:01 +0100 Subject: [PATCH 3/6] daemon: use strbuf_expand() instead of interpolate() Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- daemon.c | 109 +++++++++++++++++++++++++++---------------------------- 1 file changed, 54 insertions(+), 55 deletions(-) diff --git a/daemon.c b/daemon.c index b9ba44c58..64f60c431 100644 --- a/daemon.c +++ b/daemon.c @@ -1,7 +1,6 @@ #include "cache.h" #include "pkt-line.h" #include "exec_cmd.h" -#include "interpolate.h" #include @@ -54,26 +53,11 @@ static const char *user_path; static unsigned int timeout; static unsigned int init_timeout; -/* - * Static table for now. Ugh. - * Feel free to make dynamic as needed. - */ -#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 char *hostname; +static char *canon_hostname; +static char *ip_address; +static char *tcp_port; +static char *directory; static void logreport(int priority, const char *err, va_list params) { @@ -163,7 +147,7 @@ static int avoid_alias(char *p) } } -static char *path_ok(struct interp *itable) +static char *path_ok(void) { static char rpath[PATH_MAX]; static char interp_path[PATH_MAX]; @@ -171,7 +155,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 +185,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, - interp_table, ARRAY_SIZE(interp_table)); + strbuf_expand(&expanded_path, interpolated_path, + 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 +230,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 +296,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(struct daemon_service *service) { const char *path; int enabled = service->enabled; - loginfo("Request %s for '%s'", - service->name, - itable[INTERP_SLOT_DIR].value); + loginfo("Request %s for '%s'", service->name, directory); if (!enabled && !service->overridable) { logerror("'%s': service not enabled.", service->name); @@ -314,7 +309,7 @@ static int run_service(struct interp *itable, struct daemon_service *service) return -1; } - if (!(path = path_ok(itable))) + if (!(path = path_ok())) return -1; /* @@ -413,9 +408,8 @@ 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; @@ -433,9 +427,11 @@ 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 */ @@ -444,14 +440,14 @@ static void parse_extra_args(struct interp *table, char *extra_args, int buflen) } } -static void fill_in_extra_table_entries(struct interp *itable) +static void fill_in_extra_table_entries(void) { 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 +466,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, - INTERP_SLOT_CANON_HOST, ai->ai_canonname); - interp_set_entry(interp_table, - INTERP_SLOT_IP, addrbuf); + free(canon_hostname); + canon_hostname = xstrdup(ai->ai_canonname); + free(ip_address); + ip_address = xstrdup(addrbuf); break; } freeaddrinfo(ai0); @@ -493,7 +489,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 +500,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); - interp_set_entry(interp_table, INTERP_SLOT_IP, addrbuf); + free(canon_hostname); + canon_hostname = xstrdup(hent->h_name); + free(ip_address); + ip_address = xstrdup(addrbuf); } #endif } @@ -557,15 +555,16 @@ static int execute(struct sockaddr *addr) pktlen--; } - /* - * Initialize the path interpolation table for this connection. - */ - interp_clear_table(interp_table, ARRAY_SIZE(interp_table)); - interp_set_entry(interp_table, INTERP_SLOT_PERCENT, "%"); + free(hostname); + free(canon_hostname); + free(ip_address); + free(tcp_port); + free(directory); + hostname = canon_hostname = ip_address = tcp_port = directory = NULL; if (len != pktlen) { - parse_extra_args(interp_table, line + len + 1, pktlen - len - 1); - fill_in_extra_table_entries(interp_table); + parse_extra_args(line + len + 1, pktlen - len - 1); + fill_in_extra_table_entries(); } for (i = 0; i < ARRAY_SIZE(daemon_service); i++) { @@ -578,9 +577,9 @@ 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, - INTERP_SLOT_DIR, line + namelen + 5); - return run_service(interp_table, s); + free(directory); + directory = xstrdup(line + namelen + 5); + return run_service(s); } } From d433ed0bb49b947e3bb05d6474cf328c75ffa57d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 23 Nov 2008 00:19:09 +0100 Subject: [PATCH 4/6] daemon: inline fill_in_extra_table_entries() Having fill_in_extra_table_entries() as a separate function has no advantage -- a function with no parameters and return values might as well be an anonymous block of code. Its name still refers to the table of interpolate() which has been removed earlier, so it's better to inline it at its only call site. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- daemon.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/daemon.c b/daemon.c index 64f60c431..fbf61ca4d 100644 --- a/daemon.c +++ b/daemon.c @@ -414,6 +414,7 @@ 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; @@ -438,11 +439,6 @@ static void parse_extra_args(char *extra_args, int buflen) extra_args = val + vallen; } } -} - -static void fill_in_extra_table_entries(void) -{ - char *hp; /* * Replace literal host with lowercase-ized hostname. @@ -562,10 +558,8 @@ static int execute(struct sockaddr *addr) free(directory); hostname = canon_hostname = ip_address = tcp_port = directory = NULL; - if (len != pktlen) { + if (len != pktlen) parse_extra_args(line + len + 1, pktlen - len - 1); - fill_in_extra_table_entries(); - } for (i = 0; i < ARRAY_SIZE(daemon_service); i++) { struct daemon_service *s = &(daemon_service[i]); From a47551c3828f81ec88d5b0b3d05887f1a7a4233a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 23 Nov 2008 00:21:52 +0100 Subject: [PATCH 5/6] daemon: deglobalize variable 'directory' Remove the global variable 'directory' and pass it as a parameter of the two functions that use it instead, (almost) restoring their interface to how it was before 49ba83fb67d9e447b86953965ce5f949c6a93b81. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- daemon.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/daemon.c b/daemon.c index fbf61ca4d..1cef3098d 100644 --- a/daemon.c +++ b/daemon.c @@ -57,7 +57,6 @@ static char *hostname; static char *canon_hostname; static char *ip_address; static char *tcp_port; -static char *directory; static void logreport(int priority, const char *err, va_list params) { @@ -147,7 +146,7 @@ static int avoid_alias(char *p) } } -static char *path_ok(void) +static char *path_ok(char *directory) { static char rpath[PATH_MAX]; static char interp_path[PATH_MAX]; @@ -296,12 +295,12 @@ static int git_daemon_config(const char *var, const char *value, void *cb) return 0; } -static int run_service(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'", service->name, directory); + loginfo("Request %s for '%s'", service->name, dir); if (!enabled && !service->overridable) { logerror("'%s': service not enabled.", service->name); @@ -309,7 +308,7 @@ static int run_service(struct daemon_service *service) return -1; } - if (!(path = path_ok())) + if (!(path = path_ok(dir))) return -1; /* @@ -555,8 +554,7 @@ static int execute(struct sockaddr *addr) free(canon_hostname); free(ip_address); free(tcp_port); - free(directory); - hostname = canon_hostname = ip_address = tcp_port = directory = NULL; + hostname = canon_hostname = ip_address = tcp_port = NULL; if (len != pktlen) parse_extra_args(line + len + 1, pktlen - len - 1); @@ -571,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. */ - free(directory); - directory = xstrdup(line + namelen + 5); - return run_service(s); + return run_service(line + namelen + 5, s); } } From 7de1950cb28cee7d6b1e9cdaf22f30ddcdc5bd01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 23 Nov 2008 00:16:59 +0100 Subject: [PATCH 6/6] remove the unused files interpolate.c and interpolate.h Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- Makefile | 1 - interpolate.c | 103 -------------------------------------------------- interpolate.h | 26 ------------- 3 files changed, 130 deletions(-) delete mode 100644 interpolate.c delete mode 100644 interpolate.h diff --git a/Makefile b/Makefile index 35adafa01..54e074524 100644 --- a/Makefile +++ b/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 diff --git a/interpolate.c b/interpolate.c deleted file mode 100644 index 7f03bd99c..000000000 --- a/interpolate.c +++ /dev/null @@ -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; -} diff --git a/interpolate.h b/interpolate.h deleted file mode 100644 index 77407e67d..000000000 --- a/interpolate.h +++ /dev/null @@ -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 */