{"thread":{"id":"5721","subject":"[PATCH] Removed memory leaks from interpolation table uses.","startedAt":"2006-09-27T16:16:10Z","lastAt":"2006-09-27T19:42:52Z","messageCount":2,"participants":["Jon Loeliger","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"27795","messageId":"E1GSc4w-0005cu-I3@jdl.com","threadId":"5721","inReplyTo":null,"subject":"[PATCH] Removed memory leaks from interpolation table uses.","fromName":"Jon Loeliger","fromEmail":"jdl@jdl.com","sentAt":"2006-09-27T16:16:10Z","receivedAt":"2006-09-27T16:16:10Z","isPatch":true,"sender":{"key":"jdl@jdl.com","avatar":"https://gravatar.com/avatar/75ce9a10b151acd2c28ec4ab2136dba7b2ff1634530bd04b155981a749d08a64?d=mp&s=160"},"body":"Clarified that parse_extra_args()s results in\ninterpolation table entries.\nRemoved a few trailing whitespace occurrences.\n\nSigned-off-by: Jon Loeliger <jdl@jdl.com>\n\n---\n\nJunio,\n\nThis is on top of my previous cleanup [PATCH Rev 3].\n\nBTW, In that previous patch of mine, this line:\n\n        -\tand %D for the absolute path of the named repository.\t\n\nhas trailing blanks in my patch that must be removed in order\nto apply it correctly to the HEAD of git.  The _previous_ patch\nclearly was (correctly) applied with --whitepace=strip!\n\n*sigh*\n\nThanks,\njdl\n\n\n\n daemon.c      |   56 ++++++++++++++++++++++++++++++++++----------------------\n interpolate.c |   26 ++++++++++++++++++++++++++\n interpolate.h |    3 +++\n 3 files changed, 63 insertions(+), 22 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 260f0cf..46c7fa2 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -71,7 +71,7 @@ static struct interp interp_table[] = {\n \t{ \"%IP\", 0},\n \t{ \"%P\", 0},\n \t{ \"%D\", 0},\n-\t{ \"%%\", \"%\"},\n+\t{ \"%%\", 0},\n };\n \n \n@@ -398,7 +398,11 @@ static void make_service_overridable(con\n \tdie(\"No such service %s\", name);\n }\n \n-static void parse_extra_args(char *extra_args, int buflen)\n+/*\n+ * Separate the \"extra args\" information as supplied by the client connection.\n+ * Any resulting data is squirrelled away in the given interpolation table.\n+ */\n+static void parse_extra_args(struct interp *table, char *extra_args, int buflen)\n {\n \tchar *val;\n \tint vallen;\n@@ -410,18 +414,17 @@ static void parse_extra_args(char *extra\n \t\t\tval = extra_args + 5;\n \t\t\tvallen = strlen(val) + 1;\n \t\t\tif (*val) {\n-\t\t\t\tchar *port;\n-\t\t\t\tchar *save = xmalloc(vallen);\t/* FIXME: Leak */\n-\n-\t\t\t\tinterp_table[INTERP_SLOT_HOST].value = save;\n-\t\t\t\tstrlcpy(save, val, vallen);\n-\t\t\t\tport = strrchr(save, ':');\n+\t\t\t\t/* Split <host>:<port> at colon. */\n+\t\t\t\tchar *host = val;\n+\t\t\t\tchar *port = strrchr(host, ':');\n \t\t\t\tif (port) {\n \t\t\t\t\t*port = 0;\n \t\t\t\t\tport++;\n-\t\t\t\t\tinterp_table[INTERP_SLOT_PORT].value = port;\n+\t\t\t\t\tinterp_set_entry(table, INTERP_SLOT_PORT, port);\n \t\t\t\t}\n+\t\t\t\tinterp_set_entry(table, INTERP_SLOT_HOST, host);\n \t\t\t}\n+\n \t\t\t/* On to the next one */\n \t\t\textra_args = val + vallen;\n \t\t}\n@@ -431,8 +434,6 @@ static void parse_extra_args(char *extra\n void fill_in_extra_table_entries(struct interp *itable)\n {\n \tchar *hp;\n-\tchar *canon_host = NULL;\n-\tchar *ipaddr = NULL;\n \n \t/*\n \t * Replace literal host with lowercase-ized hostname.\n@@ -459,10 +460,12 @@ #ifndef NO_IPV6\n \t\t\tfor (ai = ai0; ai; ai = ai->ai_next) {\n \t\t\t\tstruct sockaddr_in *sin_addr = (void *)ai->ai_addr;\n \n-\t\t\t\tcanon_host = xstrdup(ai->ai_canonname);\n \t\t\t\tinet_ntop(AF_INET, &sin_addr->sin_addr,\n \t\t\t\t\t  addrbuf, sizeof(addrbuf));\n-\t\t\t\tipaddr = addrbuf;\n+\t\t\t\tinterp_set_entry(interp_table,\n+\t\t\t\t\t\t INTERP_SLOT_CANON_HOST, ai->ai_canonname);\n+\t\t\t\tinterp_set_entry(interp_table,\n+\t\t\t\t\t\t INTERP_SLOT_IP, addrbuf);\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\tfreeaddrinfo(ai0);\n@@ -476,22 +479,20 @@ #else\n \t\tstatic char addrbuf[HOST_NAME_MAX + 1];\n \n \t\thent = gethostbyname(interp_table[INTERP_SLOT_HOST].value);\n-\t\tcanon_host = xstrdup(hent->h_name);\n \n \t\tap = hent->h_addr_list;\n \t\tmemset(&sa, 0, sizeof sa);\n \t\tsa.sin_family = hent->h_addrtype;\n \t\tsa.sin_port = htons(0);\n \t\tmemcpy(&sa.sin_addr, *ap, hent->h_length);\n-\t\t\t\n+\n \t\tinet_ntop(hent->h_addrtype, &sa.sin_addr,\n \t\t\t  addrbuf, sizeof(addrbuf));\n-\t\tipaddr = addrbuf;\n+\n+\t\tinterp_set_entry(interp_table, INTERP_SLOT_CANON_HOST, hent->h_name);\n+\t\tinterp_set_entry(interp_table, INTERP_SLOT_IP, addrbuf);\n \t}\n #endif\n-\n-\tinterp_table[INTERP_SLOT_CANON_HOST].value = canon_host;\t/* FIXME: Leak */\n-\tinterp_table[INTERP_SLOT_IP].value = xstrdup(ipaddr);\t\t/* FIXME: Leak */\n }\n \n \n@@ -535,8 +536,14 @@ #endif\n \tif (len && line[len-1] == '\\n')\n \t\tline[--len] = 0;\n \n+\t/*\n+\t * Initialize the path interpolation table for this connection.\n+\t */\n+\tinterp_clear_table(interp_table, ARRAY_SIZE(interp_table));\n+\tinterp_set_entry(interp_table, INTERP_SLOT_PERCENT, \"%\");\n+\n \tif (len != pktlen) {\n-\t    parse_extra_args(line + len + 1, pktlen - len - 1);\n+\t    parse_extra_args(interp_table, line + len + 1, pktlen - len - 1);\n \t    fill_in_extra_table_entries(interp_table);\n \t}\n \n@@ -546,7 +553,12 @@ #endif\n \t\tif (!strncmp(\"git-\", line, 4) &&\n \t\t    !strncmp(s->name, line + 4, namelen) &&\n \t\t    line[namelen + 4] == ' ') {\n-\t\t\tinterp_table[INTERP_SLOT_DIR].value = line+namelen+5;\n+\t\t\t/*\n+\t\t\t * Note: The directory here is probably context sensitive,\n+\t\t\t * and might depend on the actual service being performed.\n+\t\t\t */\n+\t\t\tinterp_set_entry(interp_table,\n+\t\t\t\t\t INTERP_SLOT_DIR, line + namelen + 5);\n \t\t\treturn run_service(interp_table, s);\n \t\t}\n \t}\n@@ -982,7 +994,7 @@ int main(int argc, char **argv)\n \t\t    char *ph = listen_addr = xmalloc(strlen(arg + 9) + 1);\n \t\t    while (*p)\n \t\t\t*ph++ = tolower(*p++);\n-\t\t    *ph = 0;\t\t    \n+\t\t    *ph = 0;\n \t\t    continue;\n \t\t}\n \t\tif (!strncmp(arg, \"--port=\", 7)) {\ndiff --git a/interpolate.c b/interpolate.c\nindex d82f1b5..8357742 100644\n--- a/interpolate.c\n+++ b/interpolate.c\n@@ -4,9 +4,35 @@\n \n #include <string.h>\n \n+#include \"git-compat-util.h\"\n #include \"interpolate.h\"\n \n \n+void interp_set_entry(struct interp *table, int slot, char *value)\n+{\n+\tchar *oldval = table[slot].value;\n+\tchar *newval = value;\n+\n+\tif (oldval)\n+\t\tfree(oldval);\n+\n+\tif (value)\n+\t\tnewval = xstrdup(value);\n+\n+\ttable[slot].value = newval;\n+}\n+\n+\n+void interp_clear_table(struct interp *table, int ninterps)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < ninterps; i++) {\n+\t\tinterp_set_entry(table, i, NULL);\n+\t}\n+}\n+\n+\n /*\n  * Convert a NUL-terminated string in buffer orig\n  * into the supplied buffer, result, whose length is reslen,\ndiff --git a/interpolate.h b/interpolate.h\nindex 00c63a5..7f386bd 100644\n--- a/interpolate.h\n+++ b/interpolate.h\n@@ -11,6 +11,9 @@ struct interp {\n \tchar *value;\n };\n \n+extern void interp_set_entry(struct interp *table, int slot, char *value);\n+extern void interp_clear_table(struct interp *table, int ninterps);\n+\n extern int interpolate(char *result, int reslen,\n \t\t       char *orig,\n \t\t       struct interp *interps, int ninterps);\n-- \n1.4.2.1.g85d8-dirty\n"},{"id":"27809","messageId":"7vven99jur.fsf@assigned-by-dhcp.cox.net","threadId":"5721","inReplyTo":"E1GSc4w-0005cu-I3@jdl.com","subject":"Re: [PATCH] Removed memory leaks from interpolation table uses.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-27T19:42:52Z","receivedAt":"2006-09-27T19:42:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jon Loeliger <jdl@jdl.com> writes:\n\n> Junio,\n>\n> This is on top of my previous cleanup [PATCH Rev 3].\n\nThanks.\n\n> BTW, In that previous patch of mine, this line:\n>\n>         -\tand %D for the absolute path of the named repository.\t\n>\n> has trailing blanks in my patch that must be removed in order\n> to apply it correctly to the HEAD of git.  The _previous_ patch\n> clearly was (correctly) applied with --whitepace=strip!\n\nYes, I usually use --whitespace=strip.  The only time I\ndeliberately had to turn it off was to apply patches to diff\noutput test vectors.\n\nHopefully the recent addition to \"diff --color\" output to highlight\nthese potential whitespace errors would help spot them before\nmaking commits.\n"}]}