{"thread":{"id":"17470","subject":"[PATCH] http-push: refactor request url creation","startedAt":"2009-01-30T23:51:55Z","lastAt":"2009-02-01T22:45:59Z","messageCount":3,"participants":["Tay Ray Chuan","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"102661","messageId":"4983929B.2010901@gmail.com","threadId":"17470","inReplyTo":null,"subject":"[PATCH] http-push: refactor request url creation","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-01-30T23:51:55Z","receivedAt":"2009-01-30T23:51:55Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Currently, functions that deal with objects on the remote repository\nhave to allocate and do strcpys to generate the URL.\n\nThis patch saves them this trouble, by providing two functions,\n\"append_remote_object_url\" and \"get_remote_object_url\".\n\nBoth generate a URL, with either the object's 2-digit hex directory\n(eg. /objects/a1/), or the complete object location (eg.\n/objects/a1/b2).\n\nHowever, they differ in that \"append_remote_object_url\" appends this\nURL to a strbuf, while \"get_remote_object_url\" wraps around the former\nand returns the URL directly in char*. Users usually would use\n\"get_remote_object_url\", but may find \"append_remote_object_url\"\nuseful if they require further string operations on the URL.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\nAcked-by: Junio C Hamano <gitster@pobox.com>\n---\n\n* renamed only_two_digit_postfix in original patch to only_two_digit_prefix\n* updated with Junio's changes (if (...) and fix memory leak)\n* updated with Junio's double interface\n* added back use of temporary string \"url\" in \"start_fetch_loose\"\n* rebased and generated on master (a34a9db)\n* split \"append_remote_object_url\" signature across 3 lines at Dscho's suggestion\n\n http-push.c |   62 +++++++++++++++++++++++-----------------------------------\n 1 files changed, 25 insertions(+), 37 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 59037df..ba217fc 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -209,6 +209,22 @@ static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum d\n \treturn dav_headers;\n }\n\n+static void append_remote_object_url(struct strbuf *buf, const char *url,\n+\t\t\t\t     const char *hex,\n+\t\t\t\t     int only_two_digit_prefix)\n+{\n+\tstrbuf_addf(buf, \"%sobjects/%.*s/\", url, 2, hex);\n+\tif (!only_two_digit_prefix)\n+\t\tstrbuf_addf(buf, \"%s\", hex+2);\n+}\n+\n+static char *get_remote_object_url(const char *url, const char *hex, int only_two_digit_prefix)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tappend_remote_object_url(&buf, url, hex, only_two_digit_prefix);\n+\treturn strbuf_detach(&buf, NULL);\n+}\n+\n static void finish_request(struct transfer_request *request);\n static void release_request(struct transfer_request *request);\n\n@@ -255,7 +271,6 @@ static void start_fetch_loose(struct transfer_request *request)\n \tchar *filename;\n \tchar prevfile[PATH_MAX];\n \tchar *url;\n-\tchar *posn;\n \tint prevlocal;\n \tunsigned char prev_buf[PREV_BUF_SIZE];\n \tssize_t prev_read = 0;\n@@ -305,17 +320,8 @@ static void start_fetch_loose(struct transfer_request *request)\n\n \tgit_SHA1_Init(&request->c);\n\n-\turl = xmalloc(strlen(remote->url) + 50);\n-\trequest->url = xmalloc(strlen(remote->url) + 50);\n-\tstrcpy(url, remote->url);\n-\tposn = url + strlen(remote->url);\n-\tstrcpy(posn, \"objects/\");\n-\tposn += 8;\n-\tmemcpy(posn, hex, 2);\n-\tposn += 2;\n-\t*(posn++) = '/';\n-\tstrcpy(posn, hex + 2);\n-\tstrcpy(request->url, url);\n+\turl = get_remote_object_url(remote->url, hex, 0);\n+\trequest->url = xstrdup(url);\n\n \t/* If a previous temp file is present, process what was already\n \t   fetched. */\n@@ -388,16 +394,8 @@ static void start_mkcol(struct transfer_request *request)\n {\n \tchar *hex = sha1_to_hex(request->obj->sha1);\n \tstruct active_request_slot *slot;\n-\tchar *posn;\n\n-\trequest->url = xmalloc(strlen(remote->url) + 13);\n-\tstrcpy(request->url, remote->url);\n-\tposn = request->url + strlen(remote->url);\n-\tstrcpy(posn, \"objects/\");\n-\tposn += 8;\n-\tmemcpy(posn, hex, 2);\n-\tposn += 2;\n-\tstrcpy(posn, \"/\");\n+\trequest->url = get_remote_object_url(remote->url, hex, 1);\n\n \tslot = get_active_slot();\n \tslot->callback_func = process_response;\n@@ -512,7 +510,7 @@ static void start_put(struct transfer_request *request)\n {\n \tchar *hex = sha1_to_hex(request->obj->sha1);\n \tstruct active_request_slot *slot;\n-\tchar *posn;\n+\tstruct strbuf buf = STRBUF_INIT;\n \tenum object_type type;\n \tchar hdr[50];\n \tvoid *unpacked;\n@@ -551,21 +549,13 @@ static void start_put(struct transfer_request *request)\n\n \trequest->buffer.buf.len = stream.total_out;\n\n-\trequest->url = xmalloc(strlen(remote->url) +\n-\t\t\t       strlen(request->lock->token) + 51);\n-\tstrcpy(request->url, remote->url);\n-\tposn = request->url + strlen(remote->url);\n-\tstrcpy(posn, \"objects/\");\n-\tposn += 8;\n-\tmemcpy(posn, hex, 2);\n-\tposn += 2;\n-\t*(posn++) = '/';\n-\tstrcpy(posn, hex + 2);\n-\trequest->dest = xmalloc(strlen(request->url) + 14);\n-\tsprintf(request->dest, \"Destination: %s\", request->url);\n-\tposn += 38;\n-\t*(posn++) = '_';\n-\tstrcpy(posn, request->lock->token);\n+\tstrbuf_addstr(&buf, \"Destination: \");\n+\tappend_remote_object_url(&buf, remote->url, hex, 0);\n+\trequest->dest = strbuf_detach(&buf, NULL);\n+\n+\tappend_remote_object_url(&buf, remote->url, hex, 0);\n+\tstrbuf_addstr(&buf, request->lock->token);\n+\trequest->url = strbuf_detach(&buf, NULL);\n\n \tslot = get_active_slot();\n \tslot->callback_func = process_response;\n-- \n1.6.1.2.254.g2f9c7\n"},{"id":"102730","messageId":"7vmyd72cx6.fsf@gitster.siamese.dyndns.org","threadId":"17470","inReplyTo":"4983929B.2010901@gmail.com","subject":"Re: [PATCH] http-push: refactor request url creation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-01T00:55:49Z","receivedAt":"2009-02-01T00:55:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan <rctay89@gmail.com> writes:\n\n> Currently, functions that deal with objects on the remote repository\n> have to allocate and do strcpys to generate the URL.\n>\n> This patch saves them this trouble, by providing two functions,\n> \"append_remote_object_url\" and \"get_remote_object_url\".\n>\n> Both generate a URL, with either the object's 2-digit hex directory\n> (eg. /objects/a1/), or the complete object location (eg.\n> /objects/a1/b2).\n>\n> However, they differ in that \"append_remote_object_url\" appends this\n> URL to a strbuf, while \"get_remote_object_url\" wraps around the former\n> and returns the URL directly in char*. Users usually would use\n> \"get_remote_object_url\", but may find \"append_remote_object_url\"\n> useful if they require further string operations on the URL.\n>\n> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n> Acked-by: Junio C Hamano <gitster@pobox.com>\n\nThanks, I'll queue this to 'pu' for now, but please stop randomly adding\n\"Acked-by\" from other people, unless you conferred with them on the\nexact version of your patch you are submitting.\n\nIf somebody said \"The version that I reviewed looked sensible\", it becomes\nirrelevant after you changed your patch in response to people's comments,\nexactly because what you are sending is different from what they reviewed.\nIt is up to them, not you, to see if the issues raised in their comments\nare addressed in the new patch to their satisfaction.\n"},{"id":"102791","messageId":"alpine.DEB.1.00.0902012341310.3586@pacific.mpi-cbg.de","threadId":"17470","inReplyTo":"4983929B.2010901@gmail.com","subject":"Re: [PATCH] http-push: refactor request url creation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-01T22:45:59Z","receivedAt":"2009-02-01T22:45:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 31 Jan 2009, Tay Ray Chuan wrote:\n\n> * split \"append_remote_object_url\" signature across 3 lines at Dscho's suggestion\n\nThis line is longer than 80 characters (and I seem to remember that the \nrecommended maximum for emails is even less than that).\n\n> \n>  http-push.c |   62 +++++++++++++++++++++++-----------------------------------\n>  1 files changed, 25 insertions(+), 37 deletions(-)\n> \n> diff --git a/http-push.c b/http-push.c\n> index 59037df..ba217fc 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -209,6 +209,22 @@ static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum d\n>  \treturn dav_headers;\n>  }\n> \n> +static void append_remote_object_url(struct strbuf *buf, const char *url,\n> +\t\t\t\t     const char *hex,\n> +\t\t\t\t     int only_two_digit_prefix)\n> +{\n\nFine.  You changed that, although I find the indentation rather funny, \ntoo.  I would have expected one tab and then the rest of the signature.\n\n> +\tstrbuf_addf(buf, \"%sobjects/%.*s/\", url, 2, hex);\n> +\tif (!only_two_digit_prefix)\n> +\t\tstrbuf_addf(buf, \"%s\", hex+2);\n> +}\n> +\n> +static char *get_remote_object_url(const char *url, const char *hex, int only_two_digit_prefix)\n\nBut this is still too long.  And no, I will not go through your patch and \npoint out every too-long line; I'll expect you to do that yourself...\n\nCiao,\nDscho\n"}]}