{"thread":{"id":"38723","subject":"[PATCH] daemon: use strbuf for hostname info","startedAt":"2015-03-06T08:57:22Z","lastAt":"2015-03-07T10:50:37Z","messageCount":9,"participants":["René Scharfe","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"257136","messageId":"54F96BF2.5000504@web.de","threadId":"38723","inReplyTo":null,"subject":"[PATCH] daemon: use strbuf for hostname info","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-06T08:57:22Z","receivedAt":"2015-03-06T08:57:22Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Convert hostname, canon_hostname, ip_address and tcp_port to strbuf.\nThis allows to get rid of the helpers strbuf_addstr_or_null() and STRARG\nbecause a strbuf always represents a valid (initially empty) string.\nsanitize_client() becomes unused and is removed as well.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n daemon.c | 98 +++++++++++++++++++++++++++-------------------------------------\n 1 file changed, 41 insertions(+), 57 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex c3edd96..c04315e 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -56,10 +56,10 @@ static const char *user_path;\n static unsigned int timeout;\n static unsigned int init_timeout;\n \n-static char *hostname;\n-static char *canon_hostname;\n-static char *ip_address;\n-static char *tcp_port;\n+static struct strbuf hostname = STRBUF_INIT;\n+static struct strbuf canon_hostname = STRBUF_INIT;\n+static struct strbuf ip_address = STRBUF_INIT;\n+static struct strbuf tcp_port = STRBUF_INIT;\n \n static int hostname_lookup_done;\n \n@@ -68,13 +68,13 @@ static void lookup_hostname(void);\n static const char *get_canon_hostname(void)\n {\n \tlookup_hostname();\n-\treturn canon_hostname;\n+\treturn canon_hostname.buf;\n }\n \n static const char *get_ip_address(void)\n {\n \tlookup_hostname();\n-\treturn ip_address;\n+\treturn ip_address.buf;\n }\n \n static void logreport(int priority, const char *err, va_list params)\n@@ -122,12 +122,6 @@ static void NORETURN daemon_die(const char *err, va_list params)\n \texit(1);\n }\n \n-static void strbuf_addstr_or_null(struct strbuf *sb, const char *s)\n-{\n-\tif (s)\n-\t\tstrbuf_addstr(sb, s);\n-}\n-\n struct expand_path_context {\n \tconst char *directory;\n };\n@@ -138,22 +132,22 @@ static size_t expand_path(struct strbuf *sb, const char *placeholder, void *ctx)\n \n \tswitch (placeholder[0]) {\n \tcase 'H':\n-\t\tstrbuf_addstr_or_null(sb, hostname);\n+\t\tstrbuf_addbuf(sb, &hostname);\n \t\treturn 1;\n \tcase 'C':\n \t\tif (placeholder[1] == 'H') {\n-\t\t\tstrbuf_addstr_or_null(sb, get_canon_hostname());\n+\t\t\tstrbuf_addstr(sb, get_canon_hostname());\n \t\t\treturn 2;\n \t\t}\n \t\tbreak;\n \tcase 'I':\n \t\tif (placeholder[1] == 'P') {\n-\t\t\tstrbuf_addstr_or_null(sb, get_ip_address());\n+\t\t\tstrbuf_addstr(sb, get_ip_address());\n \t\t\treturn 2;\n \t\t}\n \t\tbreak;\n \tcase 'P':\n-\t\tstrbuf_addstr_or_null(sb, tcp_port);\n+\t\tstrbuf_addbuf(sb, &tcp_port);\n \t\treturn 1;\n \tcase 'D':\n \t\tstrbuf_addstr(sb, context->directory);\n@@ -301,16 +295,14 @@ static int run_access_hook(struct daemon_service *service, const char *dir, cons\n \tchar *eol;\n \tint seen_errors = 0;\n \n-#define STRARG(x) ((x) ? (x) : \"\")\n \t*arg++ = access_hook;\n \t*arg++ = service->name;\n \t*arg++ = path;\n-\t*arg++ = STRARG(hostname);\n-\t*arg++ = STRARG(get_canon_hostname());\n-\t*arg++ = STRARG(get_ip_address());\n-\t*arg++ = STRARG(tcp_port);\n+\t*arg++ = hostname.buf;\n+\t*arg++ = get_canon_hostname();\n+\t*arg++ = get_ip_address();\n+\t*arg++ = tcp_port.buf;\n \t*arg = NULL;\n-#undef STRARG\n \n \tchild.use_shell = 1;\n \tchild.argv = argv;\n@@ -556,23 +548,14 @@ static void sanitize_client_strbuf(struct strbuf *out, const char *in)\n \t\tstrbuf_setlen(out, out->len - 1);\n }\n \n-static char *sanitize_client(const char *in)\n-{\n-\tstruct strbuf out = STRBUF_INIT;\n-\tsanitize_client_strbuf(&out, in);\n-\treturn strbuf_detach(&out, NULL);\n-}\n-\n /*\n- * Like sanitize_client, but we also perform any canonicalization\n+ * Like sanitize_client_strbuf, but we also perform any canonicalization\n  * to make life easier on the admin.\n  */\n-static char *canonicalize_client(const char *in)\n+static void canonicalize_client_strbuf(struct strbuf *out, const char *in)\n {\n-\tstruct strbuf out = STRBUF_INIT;\n-\tsanitize_client_strbuf(&out, in);\n-\tstrbuf_tolower(&out);\n-\treturn strbuf_detach(&out, NULL);\n+\tsanitize_client_strbuf(out, in);\n+\tstrbuf_tolower(out);\n }\n \n /*\n@@ -595,11 +578,11 @@ static void parse_host_arg(char *extra_args, int buflen)\n \t\t\t\tchar *port;\n \t\t\t\tparse_host_and_port(val, &host, &port);\n \t\t\t\tif (port) {\n-\t\t\t\t\tfree(tcp_port);\n-\t\t\t\t\ttcp_port = sanitize_client(port);\n+\t\t\t\t\tstrbuf_reset(&tcp_port);\n+\t\t\t\t\tsanitize_client_strbuf(&tcp_port, port);\n \t\t\t\t}\n-\t\t\t\tfree(hostname);\n-\t\t\t\thostname = canonicalize_client(host);\n+\t\t\t\tstrbuf_reset(&hostname);\n+\t\t\t\tcanonicalize_client_strbuf(&hostname, host);\n \t\t\t\thostname_lookup_done = 0;\n \t\t\t}\n \n@@ -616,7 +599,7 @@ static void parse_host_arg(char *extra_args, int buflen)\n  */\n static void lookup_hostname(void)\n {\n-\tif (!hostname_lookup_done && hostname) {\n+\tif (!hostname_lookup_done && hostname.len) {\n #ifndef NO_IPV6\n \t\tstruct addrinfo hints;\n \t\tstruct addrinfo *ai;\n@@ -626,19 +609,21 @@ static void lookup_hostname(void)\n \t\tmemset(&hints, 0, sizeof(hints));\n \t\thints.ai_flags = AI_CANONNAME;\n \n-\t\tgai = getaddrinfo(hostname, NULL, &hints, &ai);\n+\t\tgai = getaddrinfo(hostname.buf, NULL, &hints, &ai);\n \t\tif (!gai) {\n \t\t\tstruct sockaddr_in *sin_addr = (void *)ai->ai_addr;\n \n \t\t\tinet_ntop(AF_INET, &sin_addr->sin_addr,\n \t\t\t\t  addrbuf, sizeof(addrbuf));\n-\t\t\tfree(ip_address);\n-\t\t\tip_address = xstrdup(addrbuf);\n+\t\t\tstrbuf_reset(&ip_address);\n+\t\t\tstrbuf_addstr(&ip_address, addrbuf);\n \n-\t\t\tfree(canon_hostname);\n-\t\t\tcanon_hostname = ai->ai_canonname ?\n-\t\t\t\tsanitize_client(ai->ai_canonname) :\n-\t\t\t\txstrdup(ip_address);\n+\t\t\tstrbuf_reset(&canon_hostname);\n+\t\t\tif (ai->ai_canonname)\n+\t\t\t\tsanitize_client_strbuf(&canon_hostname,\n+\t\t\t\t\t\t       ai->ai_canonname);\n+\t\t\telse\n+\t\t\t\tstrbuf_addbuf(&canon_hostname, &ip_address);\n \n \t\t\tfreeaddrinfo(ai);\n \t\t}\n@@ -648,7 +633,7 @@ static void lookup_hostname(void)\n \t\tchar **ap;\n \t\tstatic char addrbuf[HOST_NAME_MAX + 1];\n \n-\t\thent = gethostbyname(hostname);\n+\t\thent = gethostbyname(hostname.buf);\n \t\tif (hent) {\n \t\t\tap = hent->h_addr_list;\n \t\t\tmemset(&sa, 0, sizeof sa);\n@@ -659,10 +644,10 @@ static void lookup_hostname(void)\n \t\t\tinet_ntop(hent->h_addrtype, &sa.sin_addr,\n \t\t\t\t  addrbuf, sizeof(addrbuf));\n \n-\t\t\tfree(canon_hostname);\n-\t\t\tcanon_hostname = sanitize_client(hent->h_name);\n-\t\t\tfree(ip_address);\n-\t\t\tip_address = xstrdup(addrbuf);\n+\t\t\tstrbuf_reset(&canon_hostname);\n+\t\t\tsanitize_client_strbuf(&canon_hostname, hent->h_name);\n+\t\t\tstrbuf_reset(&ip_address);\n+\t\t\tstrbuf_addstr(&ip_address, addrbuf);\n \t\t}\n #endif\n \t\thostname_lookup_done = 1;\n@@ -693,11 +678,10 @@ static int execute(void)\n \t\tpktlen--;\n \t}\n \n-\tfree(hostname);\n-\tfree(canon_hostname);\n-\tfree(ip_address);\n-\tfree(tcp_port);\n-\thostname = canon_hostname = ip_address = tcp_port = NULL;\n+\tstrbuf_reset(&hostname);\n+\tstrbuf_reset(&canon_hostname);\n+\tstrbuf_reset(&ip_address);\n+\tstrbuf_reset(&tcp_port);\n \n \tif (len != pktlen)\n \t\tparse_host_arg(line + len + 1, pktlen - len - 1);\n-- \n2.3.1\n"},{"id":"257197","messageId":"20150306210627.GA24267@peff.net","threadId":"38723","inReplyTo":"54F96BF2.5000504@web.de","subject":"Re: [PATCH] daemon: use strbuf for hostname info","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-06T21:06:27Z","receivedAt":"2015-03-06T21:06:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 06, 2015 at 09:57:22AM +0100, René Scharfe wrote:\n\n> Convert hostname, canon_hostname, ip_address and tcp_port to strbuf.\n> This allows to get rid of the helpers strbuf_addstr_or_null() and STRARG\n> because a strbuf always represents a valid (initially empty) string.\n> sanitize_client() becomes unused and is removed as well.\n\nMakes sense. I had a feeling that we might have cared about NULL versus\nthe empty string somewhere, but I did not see it in the patch below, so\nI think it is fine.\n\n> -static char *sanitize_client(const char *in)\n> -{\n> -\tstruct strbuf out = STRBUF_INIT;\n> -\tsanitize_client_strbuf(&out, in);\n> -\treturn strbuf_detach(&out, NULL);\n> -}\n\nNot a big deal, but do we want to rename sanitize_client_strbuf to\nsanitize_client? It only had the unwieldy name to distinguish it from\nthis one.\n\n>  \t\t\t\tif (port) {\n> -\t\t\t\t\tfree(tcp_port);\n> -\t\t\t\t\ttcp_port = sanitize_client(port);\n> +\t\t\t\t\tstrbuf_reset(&tcp_port);\n> +\t\t\t\t\tsanitize_client_strbuf(&tcp_port, port);\n\nThe equivalent of free() is strbuf_release(). I think it is reasonable\nto strbuf_reset here, since we are about to write into it again anyway\n(though I doubt it happens much in practice, since that would imply\nmultiple `host=` segments sent by the client). But later...\n\n> -\tfree(hostname);\n> -\tfree(canon_hostname);\n> -\tfree(ip_address);\n> -\tfree(tcp_port);\n> -\thostname = canon_hostname = ip_address = tcp_port = NULL;\n> +\tstrbuf_reset(&hostname);\n> +\tstrbuf_reset(&canon_hostname);\n> +\tstrbuf_reset(&ip_address);\n> +\tstrbuf_reset(&tcp_port);\n\nThese probably want to all be strbuf_release(). Again, I doubt it\nmatters much because this is a forked daemon serving only a single\nrequest (so they'll get freed by the OS soon anyway), but I think\nfreeing the memory here follows the original intent.\n\n-Peff\n"},{"id":"257214","messageId":"54FA4446.5050103@web.de","threadId":"38723","inReplyTo":"20150306210627.GA24267@peff.net","subject":"Re: [PATCH] daemon: use strbuf for hostname info","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-07T00:20:22Z","receivedAt":"2015-03-07T00:20:22Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 06.03.2015 um 22:06 schrieb Jeff King:\n> On Fri, Mar 06, 2015 at 09:57:22AM +0100, René Scharfe wrote:\n> \n>> Convert hostname, canon_hostname, ip_address and tcp_port to strbuf.\n>> This allows to get rid of the helpers strbuf_addstr_or_null() and STRARG\n>> because a strbuf always represents a valid (initially empty) string.\n>> sanitize_client() becomes unused and is removed as well.\n> \n> Makes sense. I had a feeling that we might have cared about NULL versus\n> the empty string somewhere, but I did not see it in the patch below, so\n> I think it is fine.\n> \n>> -static char *sanitize_client(const char *in)\n>> -{\n>> -\tstruct strbuf out = STRBUF_INIT;\n>> -\tsanitize_client_strbuf(&out, in);\n>> -\treturn strbuf_detach(&out, NULL);\n>> -}\n> \n> Not a big deal, but do we want to rename sanitize_client_strbuf to\n> sanitize_client? It only had the unwieldy name to distinguish it from\n> this one.\n\nA patch would look like this.  The result is shorter, but no win in\nterms of vertical space (number of lines).\n\n-- >8 --\nSubject: daemon: drop _strbuf suffix of sanitize and canonicalize functions\n\nNow that only the strbuf variants of the functions remain, remove the\n\"_strbuf\" part from their names.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n daemon.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex c04315e..0412f8c 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -534,7 +534,7 @@ static void parse_host_and_port(char *hostport, char **host,\n  * trailing and leading dots, which means that the client cannot escape\n  * our base path via \"..\" traversal.\n  */\n-static void sanitize_client_strbuf(struct strbuf *out, const char *in)\n+static void sanitize_client(struct strbuf *out, const char *in)\n {\n \tfor (; *in; in++) {\n \t\tif (*in == '/')\n@@ -549,12 +549,12 @@ static void sanitize_client_strbuf(struct strbuf *out, const char *in)\n }\n \n /*\n- * Like sanitize_client_strbuf, but we also perform any canonicalization\n+ * Like sanitize_client, but we also perform any canonicalization\n  * to make life easier on the admin.\n  */\n-static void canonicalize_client_strbuf(struct strbuf *out, const char *in)\n+static void canonicalize_client(struct strbuf *out, const char *in)\n {\n-\tsanitize_client_strbuf(out, in);\n+\tsanitize_client(out, in);\n \tstrbuf_tolower(out);\n }\n \n@@ -579,10 +579,10 @@ static void parse_host_arg(char *extra_args, int buflen)\n \t\t\t\tparse_host_and_port(val, &host, &port);\n \t\t\t\tif (port) {\n \t\t\t\t\tstrbuf_reset(&tcp_port);\n-\t\t\t\t\tsanitize_client_strbuf(&tcp_port, port);\n+\t\t\t\t\tsanitize_client(&tcp_port, port);\n \t\t\t\t}\n \t\t\t\tstrbuf_reset(&hostname);\n-\t\t\t\tcanonicalize_client_strbuf(&hostname, host);\n+\t\t\t\tcanonicalize_client(&hostname, host);\n \t\t\t\thostname_lookup_done = 0;\n \t\t\t}\n \n@@ -620,8 +620,8 @@ static void lookup_hostname(void)\n \n \t\t\tstrbuf_reset(&canon_hostname);\n \t\t\tif (ai->ai_canonname)\n-\t\t\t\tsanitize_client_strbuf(&canon_hostname,\n-\t\t\t\t\t\t       ai->ai_canonname);\n+\t\t\t\tsanitize_client(&canon_hostname,\n+\t\t\t\t\t\tai->ai_canonname);\n \t\t\telse\n \t\t\t\tstrbuf_addbuf(&canon_hostname, &ip_address);\n \n@@ -645,7 +645,7 @@ static void lookup_hostname(void)\n \t\t\t\t  addrbuf, sizeof(addrbuf));\n \n \t\t\tstrbuf_reset(&canon_hostname);\n-\t\t\tsanitize_client_strbuf(&canon_hostname, hent->h_name);\n+\t\t\tsanitize_client(&canon_hostname, hent->h_name);\n \t\t\tstrbuf_reset(&ip_address);\n \t\t\tstrbuf_addstr(&ip_address, addrbuf);\n \t\t}\n-- \n2.3.1\n"},{"id":"257216","messageId":"54FA4C34.6080702@web.de","threadId":"38723","inReplyTo":"20150306210627.GA24267@peff.net","subject":"Re: [PATCH] daemon: use strbuf for hostname info","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-07T00:54:12Z","receivedAt":"2015-03-07T00:54:12Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 06.03.2015 um 22:06 schrieb Jeff King:\n> On Fri, Mar 06, 2015 at 09:57:22AM +0100, René Scharfe wrote:\n>>   \t\t\t\tif (port) {\n>> -\t\t\t\t\tfree(tcp_port);\n>> -\t\t\t\t\ttcp_port = sanitize_client(port);\n>> +\t\t\t\t\tstrbuf_reset(&tcp_port);\n>> +\t\t\t\t\tsanitize_client_strbuf(&tcp_port, port);\n>\n> The equivalent of free() is strbuf_release(). I think it is reasonable\n> to strbuf_reset here, since we are about to write into it again anyway\n> (though I doubt it happens much in practice, since that would imply\n> multiple `host=` segments sent by the client). But later...\n>\n>> -\tfree(hostname);\n>> -\tfree(canon_hostname);\n>> -\tfree(ip_address);\n>> -\tfree(tcp_port);\n>> -\thostname = canon_hostname = ip_address = tcp_port = NULL;\n>> +\tstrbuf_reset(&hostname);\n>> +\tstrbuf_reset(&canon_hostname);\n>> +\tstrbuf_reset(&ip_address);\n>> +\tstrbuf_reset(&tcp_port);\n>\n> These probably want to all be strbuf_release(). Again, I doubt it\n> matters much because this is a forked daemon serving only a single\n> request (so they'll get freed by the OS soon anyway), but I think\n> freeing the memory here follows the original intent.\n\nUsing a static strbuf means (in my mind) \"don't worry about freeing,\na memory leak won't happen anyway because we reuse allocations\".\nThe new code adds recycling of allocations, which I somehow expect\nin connection with static allocations where possible.  You're right\nthat using strbuf_release() would match the original code more\nstrictly.\n\nBut this block is a no-op anyway because it's the first thing we do\nto these (initially empty) variables.  That's not immediately\nobvious and should be addressed in a separate patch.\n\nRené\n"},{"id":"257217","messageId":"20150307010702.GA8202@peff.net","threadId":"38723","inReplyTo":"54FA4C34.6080702@web.de","subject":"Re: [PATCH] daemon: use strbuf for hostname info","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-07T01:07:02Z","receivedAt":"2015-03-07T01:07:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 07, 2015 at 01:54:12AM +0100, René Scharfe wrote:\n\n> >These probably want to all be strbuf_release(). Again, I doubt it\n> >matters much because this is a forked daemon serving only a single\n> >request (so they'll get freed by the OS soon anyway), but I think\n> >freeing the memory here follows the original intent.\n> \n> Using a static strbuf means (in my mind) \"don't worry about freeing,\n> a memory leak won't happen anyway because we reuse allocations\".\n> The new code adds recycling of allocations, which I somehow expect\n> in connection with static allocations where possible.  You're right\n> that using strbuf_release() would match the original code more\n> strictly.\n\nI don't mind recycling allocations like this. It's just that I think in\nthis case it makes the code actively more confusing, because we don't\nactually expect any of these allocations to be recycled, do we? We\nfork+exec for each daemon connection, which should receive exactly one\n`host=` parameter.\n\nThat being said, I don't mind it too much if the recycling stays here.\n\n> But this block is a no-op anyway because it's the first thing we do\n> to these (initially empty) variables.  That's not immediately\n> obvious and should be addressed in a separate patch.\n\nAh, yeah, just reading the diff I thought this was cleanup, not\ninitialization.\n\n-Peff\n"},{"id":"257218","messageId":"20150307010832.GB8202@peff.net","threadId":"38723","inReplyTo":"54FA4446.5050103@web.de","subject":"Re: [PATCH] daemon: use strbuf for hostname info","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-07T01:08:33Z","receivedAt":"2015-03-07T01:08:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 07, 2015 at 01:20:22AM +0100, René Scharfe wrote:\n\n> > Not a big deal, but do we want to rename sanitize_client_strbuf to\n> > sanitize_client? It only had the unwieldy name to distinguish it from\n> > this one.\n> \n> A patch would look like this.  The result is shorter, but no win in\n> terms of vertical space (number of lines).\n\nIMHO this is an improvement, though whether it is enough to merit the\ncode churn I dunno. So I'm in favor, but don't mind dropping it if\nothers disagree.\n\n-Peff\n"},{"id":"257231","messageId":"54FAD7CD.4020408@web.de","threadId":"38723","inReplyTo":"20150307010832.GB8202@peff.net","subject":"Re: [PATCH] daemon: use strbuf for hostname info","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-07T10:49:49Z","receivedAt":"2015-03-07T10:49:49Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.03.2015 um 02:08 schrieb Jeff King:\n> On Sat, Mar 07, 2015 at 01:20:22AM +0100, René Scharfe wrote:\n>\n>>> Not a big deal, but do we want to rename sanitize_client_strbuf to\n>>> sanitize_client? It only had the unwieldy name to distinguish it from\n>>> this one.\n>>\n>> A patch would look like this.  The result is shorter, but no win in\n>> terms of vertical space (number of lines).\n>\n> IMHO this is an improvement, though whether it is enough to merit the\n> code churn I dunno. So I'm in favor, but don't mind dropping it if\n> others disagree.\n\nI don't think the change justifies a separate patch, but we can squash \nit in. :)\n\nRené\n"},{"id":"257233","messageId":"54FAD7F5.4030507@web.de","threadId":"38723","inReplyTo":"54F96BF2.5000504@web.de","subject":"[PATCH v2 1/2] daemon: use strbuf for hostname info","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-07T10:50:29Z","receivedAt":"2015-03-07T10:50:29Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Convert hostname, canon_hostname, ip_address and tcp_port to strbuf.\nThis allows to get rid of the helpers strbuf_addstr_or_null() and STRARG\nbecause a strbuf always represents a valid (initially empty) string.\n\nsanitize_client() is not needed anymore and sanitize_client_strbuf()\ntakes its place and name.\n\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\nThis one is the first patch + s/_strbuf// + s/_reset/_release/.\n\n daemon.c | 98 +++++++++++++++++++++++++++-------------------------------------\n 1 file changed, 41 insertions(+), 57 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex c3edd96..265c188 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -56,10 +56,10 @@ static const char *user_path;\n static unsigned int timeout;\n static unsigned int init_timeout;\n \n-static char *hostname;\n-static char *canon_hostname;\n-static char *ip_address;\n-static char *tcp_port;\n+static struct strbuf hostname = STRBUF_INIT;\n+static struct strbuf canon_hostname = STRBUF_INIT;\n+static struct strbuf ip_address = STRBUF_INIT;\n+static struct strbuf tcp_port = STRBUF_INIT;\n \n static int hostname_lookup_done;\n \n@@ -68,13 +68,13 @@ static void lookup_hostname(void);\n static const char *get_canon_hostname(void)\n {\n \tlookup_hostname();\n-\treturn canon_hostname;\n+\treturn canon_hostname.buf;\n }\n \n static const char *get_ip_address(void)\n {\n \tlookup_hostname();\n-\treturn ip_address;\n+\treturn ip_address.buf;\n }\n \n static void logreport(int priority, const char *err, va_list params)\n@@ -122,12 +122,6 @@ static void NORETURN daemon_die(const char *err, va_list params)\n \texit(1);\n }\n \n-static void strbuf_addstr_or_null(struct strbuf *sb, const char *s)\n-{\n-\tif (s)\n-\t\tstrbuf_addstr(sb, s);\n-}\n-\n struct expand_path_context {\n \tconst char *directory;\n };\n@@ -138,22 +132,22 @@ static size_t expand_path(struct strbuf *sb, const char *placeholder, void *ctx)\n \n \tswitch (placeholder[0]) {\n \tcase 'H':\n-\t\tstrbuf_addstr_or_null(sb, hostname);\n+\t\tstrbuf_addbuf(sb, &hostname);\n \t\treturn 1;\n \tcase 'C':\n \t\tif (placeholder[1] == 'H') {\n-\t\t\tstrbuf_addstr_or_null(sb, get_canon_hostname());\n+\t\t\tstrbuf_addstr(sb, get_canon_hostname());\n \t\t\treturn 2;\n \t\t}\n \t\tbreak;\n \tcase 'I':\n \t\tif (placeholder[1] == 'P') {\n-\t\t\tstrbuf_addstr_or_null(sb, get_ip_address());\n+\t\t\tstrbuf_addstr(sb, get_ip_address());\n \t\t\treturn 2;\n \t\t}\n \t\tbreak;\n \tcase 'P':\n-\t\tstrbuf_addstr_or_null(sb, tcp_port);\n+\t\tstrbuf_addbuf(sb, &tcp_port);\n \t\treturn 1;\n \tcase 'D':\n \t\tstrbuf_addstr(sb, context->directory);\n@@ -301,16 +295,14 @@ static int run_access_hook(struct daemon_service *service, const char *dir, cons\n \tchar *eol;\n \tint seen_errors = 0;\n \n-#define STRARG(x) ((x) ? (x) : \"\")\n \t*arg++ = access_hook;\n \t*arg++ = service->name;\n \t*arg++ = path;\n-\t*arg++ = STRARG(hostname);\n-\t*arg++ = STRARG(get_canon_hostname());\n-\t*arg++ = STRARG(get_ip_address());\n-\t*arg++ = STRARG(tcp_port);\n+\t*arg++ = hostname.buf;\n+\t*arg++ = get_canon_hostname();\n+\t*arg++ = get_ip_address();\n+\t*arg++ = tcp_port.buf;\n \t*arg = NULL;\n-#undef STRARG\n \n \tchild.use_shell = 1;\n \tchild.argv = argv;\n@@ -542,7 +534,7 @@ static void parse_host_and_port(char *hostport, char **host,\n  * trailing and leading dots, which means that the client cannot escape\n  * our base path via \"..\" traversal.\n  */\n-static void sanitize_client_strbuf(struct strbuf *out, const char *in)\n+static void sanitize_client(struct strbuf *out, const char *in)\n {\n \tfor (; *in; in++) {\n \t\tif (*in == '/')\n@@ -556,23 +548,14 @@ static void sanitize_client_strbuf(struct strbuf *out, const char *in)\n \t\tstrbuf_setlen(out, out->len - 1);\n }\n \n-static char *sanitize_client(const char *in)\n-{\n-\tstruct strbuf out = STRBUF_INIT;\n-\tsanitize_client_strbuf(&out, in);\n-\treturn strbuf_detach(&out, NULL);\n-}\n-\n /*\n  * Like sanitize_client, but we also perform any canonicalization\n  * to make life easier on the admin.\n  */\n-static char *canonicalize_client(const char *in)\n+static void canonicalize_client(struct strbuf *out, const char *in)\n {\n-\tstruct strbuf out = STRBUF_INIT;\n-\tsanitize_client_strbuf(&out, in);\n-\tstrbuf_tolower(&out);\n-\treturn strbuf_detach(&out, NULL);\n+\tsanitize_client(out, in);\n+\tstrbuf_tolower(out);\n }\n \n /*\n@@ -595,11 +578,11 @@ static void parse_host_arg(char *extra_args, int buflen)\n \t\t\t\tchar *port;\n \t\t\t\tparse_host_and_port(val, &host, &port);\n \t\t\t\tif (port) {\n-\t\t\t\t\tfree(tcp_port);\n-\t\t\t\t\ttcp_port = sanitize_client(port);\n+\t\t\t\t\tstrbuf_reset(&tcp_port);\n+\t\t\t\t\tsanitize_client(&tcp_port, port);\n \t\t\t\t}\n-\t\t\t\tfree(hostname);\n-\t\t\t\thostname = canonicalize_client(host);\n+\t\t\t\tstrbuf_reset(&hostname);\n+\t\t\t\tcanonicalize_client(&hostname, host);\n \t\t\t\thostname_lookup_done = 0;\n \t\t\t}\n \n@@ -616,7 +599,7 @@ static void parse_host_arg(char *extra_args, int buflen)\n  */\n static void lookup_hostname(void)\n {\n-\tif (!hostname_lookup_done && hostname) {\n+\tif (!hostname_lookup_done && hostname.len) {\n #ifndef NO_IPV6\n \t\tstruct addrinfo hints;\n \t\tstruct addrinfo *ai;\n@@ -626,19 +609,21 @@ static void lookup_hostname(void)\n \t\tmemset(&hints, 0, sizeof(hints));\n \t\thints.ai_flags = AI_CANONNAME;\n \n-\t\tgai = getaddrinfo(hostname, NULL, &hints, &ai);\n+\t\tgai = getaddrinfo(hostname.buf, NULL, &hints, &ai);\n \t\tif (!gai) {\n \t\t\tstruct sockaddr_in *sin_addr = (void *)ai->ai_addr;\n \n \t\t\tinet_ntop(AF_INET, &sin_addr->sin_addr,\n \t\t\t\t  addrbuf, sizeof(addrbuf));\n-\t\t\tfree(ip_address);\n-\t\t\tip_address = xstrdup(addrbuf);\n+\t\t\tstrbuf_reset(&ip_address);\n+\t\t\tstrbuf_addstr(&ip_address, addrbuf);\n \n-\t\t\tfree(canon_hostname);\n-\t\t\tcanon_hostname = ai->ai_canonname ?\n-\t\t\t\tsanitize_client(ai->ai_canonname) :\n-\t\t\t\txstrdup(ip_address);\n+\t\t\tstrbuf_reset(&canon_hostname);\n+\t\t\tif (ai->ai_canonname)\n+\t\t\t\tsanitize_client(&canon_hostname,\n+\t\t\t\t\t\tai->ai_canonname);\n+\t\t\telse\n+\t\t\t\tstrbuf_addbuf(&canon_hostname, &ip_address);\n \n \t\t\tfreeaddrinfo(ai);\n \t\t}\n@@ -648,7 +633,7 @@ static void lookup_hostname(void)\n \t\tchar **ap;\n \t\tstatic char addrbuf[HOST_NAME_MAX + 1];\n \n-\t\thent = gethostbyname(hostname);\n+\t\thent = gethostbyname(hostname.buf);\n \t\tif (hent) {\n \t\t\tap = hent->h_addr_list;\n \t\t\tmemset(&sa, 0, sizeof sa);\n@@ -659,10 +644,10 @@ static void lookup_hostname(void)\n \t\t\tinet_ntop(hent->h_addrtype, &sa.sin_addr,\n \t\t\t\t  addrbuf, sizeof(addrbuf));\n \n-\t\t\tfree(canon_hostname);\n-\t\t\tcanon_hostname = sanitize_client(hent->h_name);\n-\t\t\tfree(ip_address);\n-\t\t\tip_address = xstrdup(addrbuf);\n+\t\t\tstrbuf_reset(&canon_hostname);\n+\t\t\tsanitize_client(&canon_hostname, hent->h_name);\n+\t\t\tstrbuf_reset(&ip_address);\n+\t\t\tstrbuf_addstr(&ip_address, addrbuf);\n \t\t}\n #endif\n \t\thostname_lookup_done = 1;\n@@ -693,11 +678,10 @@ static int execute(void)\n \t\tpktlen--;\n \t}\n \n-\tfree(hostname);\n-\tfree(canon_hostname);\n-\tfree(ip_address);\n-\tfree(tcp_port);\n-\thostname = canon_hostname = ip_address = tcp_port = NULL;\n+\tstrbuf_release(&hostname);\n+\tstrbuf_release(&canon_hostname);\n+\tstrbuf_release(&ip_address);\n+\tstrbuf_release(&tcp_port);\n \n \tif (len != pktlen)\n \t\tparse_host_arg(line + len + 1, pktlen - len - 1);\n-- \n2.3.1\n"},{"id":"257232","messageId":"54FAD7FD.1020102@web.de","threadId":"38723","inReplyTo":"54F96BF2.5000504@web.de","subject":"[PATCH v2 2/2] daemon: deglobalize hostname information","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2015-03-07T10:50:37Z","receivedAt":"2015-03-07T10:50:37Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Move the variables related to the client-supplied hostname into its own\nstruct, let execute() own an instance of that instead of storing the\ninformation in global variables and pass the struct to any function that\nneeds to access it as a parameter.\n\nThe lifetime of the variables is easier to see this way.  Allocated\nmemory is released within execute().  The strbufs don't have to be reset\nanymore because they are written to only once at most: parse_host_arg()\nis only called once by execute() and lookup_hostname() guards against\nbeing called twice using hostname_lookup_done.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n daemon.c | 133 +++++++++++++++++++++++++++++++++++----------------------------\n 1 file changed, 74 insertions(+), 59 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 265c188..9ee2187 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -43,9 +43,6 @@ static const char *base_path;\n static const char *interpolated_path;\n static int base_path_relaxed;\n \n-/* Flag indicating client sent extra args. */\n-static int saw_extended_args;\n-\n /* If defined, ~user notation is allowed and the string is inserted\n  * after ~user/.  E.g. a request to git://host/~alice/frotz would\n  * go to /home/alice/pub_git/frotz with --user-path=pub_git.\n@@ -56,25 +53,27 @@ static const char *user_path;\n static unsigned int timeout;\n static unsigned int init_timeout;\n \n-static struct strbuf hostname = STRBUF_INIT;\n-static struct strbuf canon_hostname = STRBUF_INIT;\n-static struct strbuf ip_address = STRBUF_INIT;\n-static struct strbuf tcp_port = STRBUF_INIT;\n-\n-static int hostname_lookup_done;\n+struct hostinfo {\n+\tstruct strbuf hostname;\n+\tstruct strbuf canon_hostname;\n+\tstruct strbuf ip_address;\n+\tstruct strbuf tcp_port;\n+\tunsigned int hostname_lookup_done:1;\n+\tunsigned int saw_extended_args:1;\n+};\n \n-static void lookup_hostname(void);\n+static void lookup_hostname(struct hostinfo *hi);\n \n-static const char *get_canon_hostname(void)\n+static const char *get_canon_hostname(struct hostinfo *hi)\n {\n-\tlookup_hostname();\n-\treturn canon_hostname.buf;\n+\tlookup_hostname(hi);\n+\treturn hi->canon_hostname.buf;\n }\n \n-static const char *get_ip_address(void)\n+static const char *get_ip_address(struct hostinfo *hi)\n {\n-\tlookup_hostname();\n-\treturn ip_address.buf;\n+\tlookup_hostname(hi);\n+\treturn hi->ip_address.buf;\n }\n \n static void logreport(int priority, const char *err, va_list params)\n@@ -124,30 +123,32 @@ static void NORETURN daemon_die(const char *err, va_list params)\n \n struct expand_path_context {\n \tconst char *directory;\n+\tstruct hostinfo *hostinfo;\n };\n \n static size_t expand_path(struct strbuf *sb, const char *placeholder, void *ctx)\n {\n \tstruct expand_path_context *context = ctx;\n+\tstruct hostinfo *hi = context->hostinfo;\n \n \tswitch (placeholder[0]) {\n \tcase 'H':\n-\t\tstrbuf_addbuf(sb, &hostname);\n+\t\tstrbuf_addbuf(sb, &hi->hostname);\n \t\treturn 1;\n \tcase 'C':\n \t\tif (placeholder[1] == 'H') {\n-\t\t\tstrbuf_addstr(sb, get_canon_hostname());\n+\t\t\tstrbuf_addstr(sb, get_canon_hostname(hi));\n \t\t\treturn 2;\n \t\t}\n \t\tbreak;\n \tcase 'I':\n \t\tif (placeholder[1] == 'P') {\n-\t\t\tstrbuf_addstr(sb, get_ip_address());\n+\t\t\tstrbuf_addstr(sb, get_ip_address(hi));\n \t\t\treturn 2;\n \t\t}\n \t\tbreak;\n \tcase 'P':\n-\t\tstrbuf_addbuf(sb, &tcp_port);\n+\t\tstrbuf_addbuf(sb, &hi->tcp_port);\n \t\treturn 1;\n \tcase 'D':\n \t\tstrbuf_addstr(sb, context->directory);\n@@ -156,7 +157,7 @@ static size_t expand_path(struct strbuf *sb, const char *placeholder, void *ctx)\n \treturn 0;\n }\n \n-static const char *path_ok(const char *directory)\n+static const char *path_ok(const char *directory, struct hostinfo *hi)\n {\n \tstatic char rpath[PATH_MAX];\n \tstatic char interp_path[PATH_MAX];\n@@ -192,11 +193,12 @@ static const char *path_ok(const char *directory)\n \t\t\tdir = rpath;\n \t\t}\n \t}\n-\telse if (interpolated_path && saw_extended_args) {\n+\telse if (interpolated_path && hi->saw_extended_args) {\n \t\tstruct strbuf expanded_path = STRBUF_INIT;\n \t\tstruct expand_path_context context;\n \n \t\tcontext.directory = directory;\n+\t\tcontext.hostinfo = hi;\n \n \t\tif (*dir != '/') {\n \t\t\t/* Allow only absolute */\n@@ -286,7 +288,8 @@ static int daemon_error(const char *dir, const char *msg)\n \n static const char *access_hook;\n \n-static int run_access_hook(struct daemon_service *service, const char *dir, const char *path)\n+static int run_access_hook(struct daemon_service *service, const char *dir,\n+\t\t\t   const char *path, struct hostinfo *hi)\n {\n \tstruct child_process child = CHILD_PROCESS_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -298,10 +301,10 @@ static int run_access_hook(struct daemon_service *service, const char *dir, cons\n \t*arg++ = access_hook;\n \t*arg++ = service->name;\n \t*arg++ = path;\n-\t*arg++ = hostname.buf;\n-\t*arg++ = get_canon_hostname();\n-\t*arg++ = get_ip_address();\n-\t*arg++ = tcp_port.buf;\n+\t*arg++ = hi->hostname.buf;\n+\t*arg++ = get_canon_hostname(hi);\n+\t*arg++ = get_ip_address(hi);\n+\t*arg++ = hi->tcp_port.buf;\n \t*arg = NULL;\n \n \tchild.use_shell = 1;\n@@ -346,7 +349,8 @@ error_return:\n \treturn -1;\n }\n \n-static int run_service(const char *dir, struct daemon_service *service)\n+static int run_service(const char *dir, struct daemon_service *service,\n+\t\t       struct hostinfo *hi)\n {\n \tconst char *path;\n \tint enabled = service->enabled;\n@@ -360,7 +364,7 @@ static int run_service(const char *dir, struct daemon_service *service)\n \t\treturn daemon_error(dir, \"service not enabled\");\n \t}\n \n-\tif (!(path = path_ok(dir)))\n+\tif (!(path = path_ok(dir, hi)))\n \t\treturn daemon_error(dir, \"no such repository\");\n \n \t/*\n@@ -396,7 +400,7 @@ static int run_service(const char *dir, struct daemon_service *service)\n \t * Optionally, a hook can choose to deny access to the\n \t * repository depending on the phase of the moon.\n \t */\n-\tif (access_hook && run_access_hook(service, dir, path))\n+\tif (access_hook && run_access_hook(service, dir, path, hi))\n \t\treturn -1;\n \n \t/*\n@@ -561,14 +565,14 @@ static void canonicalize_client(struct strbuf *out, const char *in)\n /*\n  * Read the host as supplied by the client connection.\n  */\n-static void parse_host_arg(char *extra_args, int buflen)\n+static void parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)\n {\n \tchar *val;\n \tint vallen;\n \tchar *end = extra_args + buflen;\n \n \tif (extra_args < end && *extra_args) {\n-\t\tsaw_extended_args = 1;\n+\t\thi->saw_extended_args = 1;\n \t\tif (strncasecmp(\"host=\", extra_args, 5) == 0) {\n \t\t\tval = extra_args + 5;\n \t\t\tvallen = strlen(val) + 1;\n@@ -577,13 +581,10 @@ static void parse_host_arg(char *extra_args, int buflen)\n \t\t\t\tchar *host;\n \t\t\t\tchar *port;\n \t\t\t\tparse_host_and_port(val, &host, &port);\n-\t\t\t\tif (port) {\n-\t\t\t\t\tstrbuf_reset(&tcp_port);\n-\t\t\t\t\tsanitize_client(&tcp_port, port);\n-\t\t\t\t}\n-\t\t\t\tstrbuf_reset(&hostname);\n-\t\t\t\tcanonicalize_client(&hostname, host);\n-\t\t\t\thostname_lookup_done = 0;\n+\t\t\t\tif (port)\n+\t\t\t\t\tsanitize_client(&hi->tcp_port, port);\n+\t\t\t\tcanonicalize_client(&hi->hostname, host);\n+\t\t\t\thi->hostname_lookup_done = 0;\n \t\t\t}\n \n \t\t\t/* On to the next one */\n@@ -597,9 +598,9 @@ static void parse_host_arg(char *extra_args, int buflen)\n /*\n  * Locate canonical hostname and its IP address.\n  */\n-static void lookup_hostname(void)\n+static void lookup_hostname(struct hostinfo *hi)\n {\n-\tif (!hostname_lookup_done && hostname.len) {\n+\tif (!hi->hostname_lookup_done && hi->hostname.len) {\n #ifndef NO_IPV6\n \t\tstruct addrinfo hints;\n \t\tstruct addrinfo *ai;\n@@ -609,21 +610,20 @@ static void lookup_hostname(void)\n \t\tmemset(&hints, 0, sizeof(hints));\n \t\thints.ai_flags = AI_CANONNAME;\n \n-\t\tgai = getaddrinfo(hostname.buf, NULL, &hints, &ai);\n+\t\tgai = getaddrinfo(hi->hostname.buf, NULL, &hints, &ai);\n \t\tif (!gai) {\n \t\t\tstruct sockaddr_in *sin_addr = (void *)ai->ai_addr;\n \n \t\t\tinet_ntop(AF_INET, &sin_addr->sin_addr,\n \t\t\t\t  addrbuf, sizeof(addrbuf));\n-\t\t\tstrbuf_reset(&ip_address);\n-\t\t\tstrbuf_addstr(&ip_address, addrbuf);\n+\t\t\tstrbuf_addstr(&hi->ip_address, addrbuf);\n \n-\t\t\tstrbuf_reset(&canon_hostname);\n \t\t\tif (ai->ai_canonname)\n-\t\t\t\tsanitize_client(&canon_hostname,\n+\t\t\t\tsanitize_client(&hi->canon_hostname,\n \t\t\t\t\t\tai->ai_canonname);\n \t\t\telse\n-\t\t\t\tstrbuf_addbuf(&canon_hostname, &ip_address);\n+\t\t\t\tstrbuf_addbuf(&hi->canon_hostname,\n+\t\t\t\t\t      &hi->ip_address);\n \n \t\t\tfreeaddrinfo(ai);\n \t\t}\n@@ -644,22 +644,39 @@ static void lookup_hostname(void)\n \t\t\tinet_ntop(hent->h_addrtype, &sa.sin_addr,\n \t\t\t\t  addrbuf, sizeof(addrbuf));\n \n-\t\t\tstrbuf_reset(&canon_hostname);\n-\t\t\tsanitize_client(&canon_hostname, hent->h_name);\n-\t\t\tstrbuf_reset(&ip_address);\n-\t\t\tstrbuf_addstr(&ip_address, addrbuf);\n+\t\t\tsanitize_client(&hi->canon_hostname, hent->h_name);\n+\t\t\tstrbuf_addstr(&hi->ip_address, addrbuf);\n \t\t}\n #endif\n-\t\thostname_lookup_done = 1;\n+\t\thi->hostname_lookup_done = 1;\n \t}\n }\n \n+static void hostinfo_init(struct hostinfo *hi)\n+{\n+\tmemset(hi, 0, sizeof(*hi));\n+\tstrbuf_init(&hi->hostname, 0);\n+\tstrbuf_init(&hi->canon_hostname, 0);\n+\tstrbuf_init(&hi->ip_address, 0);\n+\tstrbuf_init(&hi->tcp_port, 0);\n+}\n+\n+static void hostinfo_clear(struct hostinfo *hi)\n+{\n+\tstrbuf_release(&hi->hostname);\n+\tstrbuf_release(&hi->canon_hostname);\n+\tstrbuf_release(&hi->ip_address);\n+\tstrbuf_release(&hi->tcp_port);\n+}\n \n static int execute(void)\n {\n \tchar *line = packet_buffer;\n \tint pktlen, len, i;\n \tchar *addr = getenv(\"REMOTE_ADDR\"), *port = getenv(\"REMOTE_PORT\");\n+\tstruct hostinfo hi;\n+\n+\thostinfo_init(&hi);\n \n \tif (addr)\n \t\tloginfo(\"Connection from %s:%s\", addr, port);\n@@ -678,13 +695,8 @@ static int execute(void)\n \t\tpktlen--;\n \t}\n \n-\tstrbuf_release(&hostname);\n-\tstrbuf_release(&canon_hostname);\n-\tstrbuf_release(&ip_address);\n-\tstrbuf_release(&tcp_port);\n-\n \tif (len != pktlen)\n-\t\tparse_host_arg(line + len + 1, pktlen - len - 1);\n+\t\tparse_host_arg(&hi, line + len + 1, pktlen - len - 1);\n \n \tfor (i = 0; i < ARRAY_SIZE(daemon_service); i++) {\n \t\tstruct daemon_service *s = &(daemon_service[i]);\n@@ -697,10 +709,13 @@ static int execute(void)\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\treturn run_service(arg, s);\n+\t\t\tint rc = run_service(arg, s, &hi);\n+\t\t\thostinfo_clear(&hi);\n+\t\t\treturn rc;\n \t\t}\n \t}\n \n+\thostinfo_clear(&hi);\n \tlogerror(\"Protocol error: '%s'\", line);\n \treturn -1;\n }\n-- \n2.3.1\n"}]}