{"thread":{"id":"18195","subject":"[PATCH] http: use strbuf API in quote_ref_url","startedAt":"2009-03-07T15:40:04Z","lastAt":"2009-03-07T15:57:58Z","messageCount":2,"participants":["Tay Ray Chuan","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"107303","messageId":"49B29554.30805@gmail.com","threadId":"18195","inReplyTo":null,"subject":"[PATCH] http: use strbuf API in quote_ref_url","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-03-07T15:40:04Z","receivedAt":"2009-03-07T15:40:04Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"In addition, ''quote_ref_url'' inserts a slash between the base URL and\nremote ref path only if needed. Previously, this insertion wasn't\ncontingent on the lack of a separating slash.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\nAcked-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n http.c |   29 ++++++++++-------------------\n 1 files changed, 10 insertions(+), 19 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex cdedeb6..9de4130 100644\n--- a/http.c\n+++ b/http.c\n@@ -577,31 +577,22 @@ static inline int hex(int v)\n\n static char *quote_ref_url(const char *base, const char *ref)\n {\n+\tstruct strbuf buf = STRBUF_INIT;\n \tconst char *cp;\n-\tchar *dp, *qref;\n-\tint len, baselen, ch;\n+\tint ch;\n+\n+\tstrbuf_addstr(&buf, base);\n+\tif (strcmp(base+strlen(base)-1, \"/\") && strcmp(ref, \"/\"))\n+\t\tstrbuf_addstr(&buf, \"/\");\n\n-\tbaselen = strlen(base);\n-\tlen = baselen + 2; /* '/' after base and terminating NUL */\n-\tfor (cp = ref; (ch = *cp) != 0; cp++, len++)\n-\t\tif (needs_quote(ch))\n-\t\t\tlen += 2; /* extra two hex plus replacement % */\n-\tqref = xmalloc(len);\n-\tmemcpy(qref, base, baselen);\n-\tdp = qref + baselen;\n-\t*(dp++) = '/';\n \tfor (cp = ref; (ch = *cp) != 0; cp++) {\n-\t\tif (needs_quote(ch)) {\n-\t\t\t*dp++ = '%';\n-\t\t\t*dp++ = hex((ch >> 4) & 0xF);\n-\t\t\t*dp++ = hex(ch & 0xF);\n-\t\t}\n+\t\tif (needs_quote(ch))\n+\t\t\tstrbuf_addf(&buf, \"%%%02x\", ch);\n \t\telse\n-\t\t\t*dp++ = ch;\n+\t\t\tstrbuf_addch(&buf, *cp);\n \t}\n-\t*dp = 0;\n\n-\treturn qref;\n+\treturn strbuf_detach(&buf, NULL);\n }\n\n int http_fetch_ref(const char *base, struct ref *ref)\n-- \n1.6.2.rc1\n"},{"id":"107306","messageId":"alpine.DEB.1.00.0903071654480.10279@pacific.mpi-cbg.de","threadId":"18195","inReplyTo":"49B29554.30805@gmail.com","subject":"Re: [PATCH] http: use strbuf API in quote_ref_url","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-07T15:57:58Z","receivedAt":"2009-03-07T15:57:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 7 Mar 2009, Tay Ray Chuan wrote:\n\n> In addition, ''quote_ref_url'' inserts a slash between the base URL and\n> remote ref path only if needed. Previously, this insertion wasn't\n> contingent on the lack of a separating slash.\n> \n> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n\nI would prefer to give my ACK explicitely... :-)\n\n>  http.c |   29 ++++++++++-------------------\n>  1 files changed, 10 insertions(+), 19 deletions(-)\n> \n> diff --git a/http.c b/http.c\n> index cdedeb6..9de4130 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -577,31 +577,22 @@ static inline int hex(int v)\n> \n>  static char *quote_ref_url(const char *base, const char *ref)\n>  {\n> +\tstruct strbuf buf = STRBUF_INIT;\n>  \tconst char *cp;\n> -\tchar *dp, *qref;\n> -\tint len, baselen, ch;\n> +\tint ch;\n> +\n> +\tstrbuf_addstr(&buf, base);\n> +\tif (strcmp(base+strlen(base)-1, \"/\") && strcmp(ref, \"/\"))\n> +\t\tstrbuf_addstr(&buf, \"/\");\n\nI would not have scratched my head that much if it read like this:\n\n\tif (buf.len && buf.buf[buf.len - 1] != '/' && *ref != '/')\n\t\tstrbuf_addch(&buf, '/');\n\n>  \tfor (cp = ref; (ch = *cp) != 0; cp++) {\n> -\t\tif (needs_quote(ch)) {\n> -\t\t\t*dp++ = '%';\n> -\t\t\t*dp++ = hex((ch >> 4) & 0xF);\n> -\t\t\t*dp++ = hex(ch & 0xF);\n> -\t\t}\n> +\t\tif (needs_quote(ch))\n> +\t\t\tstrbuf_addf(&buf, \"%%%02x\", ch);\n>  \t\telse\n> -\t\t\t*dp++ = ch;\n> +\t\t\tstrbuf_addch(&buf, *cp);\n>  \t}\n\nSeems as if you could remove even the curly brackets here.\n\nOther than that, it indeed looks like an ACK from me...\n\nCiao,\nDscho\n"}]}