{"thread":{"id":"17335","subject":"[PATCH] http-push: refactor lock-related headers creation for curl requests","startedAt":"2009-01-24T02:00:22Z","lastAt":"2009-01-24T05:18:22Z","messageCount":3,"participants":["Ray Chuan","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"101722","messageId":"be6fef0d0901231800t6943b01dwbca976d5e9f3397@mail.gmail.com","threadId":"17335","inReplyTo":null,"subject":"[PATCH] http-push: refactor lock-related headers creation for curl requests","fromName":"Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-01-24T02:00:22Z","receivedAt":"2009-01-24T02:00:22Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Currently, DAV-related headers (more specifically, headers related to\nthe lock token, namely, If, Lock-Token, and Timeout) for curl requests\nare created and allocated individually, eg a \"if_header\" variable for\nthe \"If: \" header, a \"timeout_header\" variable for the \"Timeout: \"\nheader.\n\nThis patch provides a new function (\"get_dav_token_headers\") that\ncreates these header, saving methods from allocating memory, and from\nissuing a \"curl_slist_append\" call.\n\nIn part, this patch also addresses the fact that commit 753bc91\n(Remove the requirement opaquelocktoken uri scheme) did not update\nmemory allocations for DAV-related headers.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n http-push.c |   68 +++++++++++++++++++++++++++++++++-------------------------\n 1 files changed, 39 insertions(+), 29 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex cb5bf95..eca4a8e 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -177,6 +177,37 @@ struct remote_ls_ctx\n \tstruct remote_ls_ctx *parent;\n };\n\n+/* get_dav_token_headers options */\n+enum dav_header_flag {\n+\tDAV_HEADER_IF = (1u << 0),\n+\tDAV_HEADER_LOCK = (1u << 1),\n+\tDAV_HEADER_TIMEOUT = (1u << 2)\n+};\n+\n+static struct curl_slist *get_dav_token_headers(struct remote_lock\n*lock, enum dav_header_flag options) {\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct curl_slist *dav_headers = NULL;\n+\n+\tif(options & DAV_HEADER_IF) {\n+\t\tstrbuf_addf(&buf, \"If: (<%s>)\", lock->token);\n+\t\tdav_headers = curl_slist_append(dav_headers, buf.buf);\n+\t\tstrbuf_reset(&buf);\n+\t}\n+\tif(options & DAV_HEADER_LOCK) {\n+\t\tstrbuf_addf(&buf, \"Lock-Token: <%s>\", lock->token);\n+\t\tdav_headers = curl_slist_append(dav_headers, buf.buf);\n+\t\tstrbuf_reset(&buf);\n+\t}\n+\tif(options & DAV_HEADER_TIMEOUT) {\n+\t\tstrbuf_addf(&buf, \"Timeout: Second-%ld\", lock->timeout);\n+\t\tdav_headers = curl_slist_append(dav_headers, buf.buf);\n+\t\tstrbuf_reset(&buf);\n+\t}\n+\tstrbuf_release(&buf);\n+\n+\treturn dav_headers;\n+}\n+\n static void finish_request(struct transfer_request *request);\n static void release_request(struct transfer_request *request);\n\n@@ -588,18 +619,12 @@ static int refresh_lock(struct remote_lock *lock)\n {\n \tstruct active_request_slot *slot;\n \tstruct slot_results results;\n-\tchar *if_header;\n-\tchar timeout_header[25];\n-\tstruct curl_slist *dav_headers = NULL;\n+\tstruct curl_slist *dav_headers;\n \tint rc = 0;\n\n \tlock->refreshing = 1;\n\n-\tif_header = xmalloc(strlen(lock->token) + 25);\n-\tsprintf(if_header, \"If: (<%s>)\", lock->token);\n-\tsprintf(timeout_header, \"Timeout: Second-%ld\", lock->timeout);\n-\tdav_headers = curl_slist_append(dav_headers, if_header);\n-\tdav_headers = curl_slist_append(dav_headers, timeout_header);\n+\tdav_headers = get_dav_token_headers(lock, DAV_HEADER_IF | DAV_HEADER_TIMEOUT);\n\n \tslot = get_active_slot();\n \tslot->results = &results;\n@@ -622,7 +647,6 @@ static int refresh_lock(struct remote_lock *lock)\n\n \tlock->refreshing = 0;\n \tcurl_slist_free_all(dav_headers);\n-\tfree(if_header);\n\n \treturn rc;\n }\n@@ -1303,14 +1327,10 @@ static int unlock_remote(struct remote_lock *lock)\n \tstruct active_request_slot *slot;\n \tstruct slot_results results;\n \tstruct remote_lock *prev = remote->locks;\n-\tchar *lock_token_header;\n-\tstruct curl_slist *dav_headers = NULL;\n+\tstruct curl_slist *dav_headers;\n \tint rc = 0;\n\n-\tlock_token_header = xmalloc(strlen(lock->token) + 31);\n-\tsprintf(lock_token_header, \"Lock-Token: <%s>\",\n-\t\tlock->token);\n-\tdav_headers = curl_slist_append(dav_headers, lock_token_header);\n+\tdav_headers = get_dav_token_headers(lock, DAV_HEADER_LOCK);\n\n \tslot = get_active_slot();\n \tslot->results = &results;\n@@ -1331,7 +1351,6 @@ static int unlock_remote(struct remote_lock *lock)\n \t}\n\n \tcurl_slist_free_all(dav_headers);\n-\tfree(lock_token_header);\n\n \tif (remote->locks == lock) {\n \t\tremote->locks = lock->next;\n@@ -1731,13 +1750,10 @@ static int update_remote(unsigned char *sha1,\nstruct remote_lock *lock)\n {\n \tstruct active_request_slot *slot;\n \tstruct slot_results results;\n-\tchar *if_header;\n \tstruct buffer out_buffer = { STRBUF_INIT, 0 };\n-\tstruct curl_slist *dav_headers = NULL;\n+\tstruct curl_slist *dav_headers;\n\n-\tif_header = xmalloc(strlen(lock->token) + 25);\n-\tsprintf(if_header, \"If: (<%s>)\", lock->token);\n-\tdav_headers = curl_slist_append(dav_headers, if_header);\n+\tdav_headers = get_dav_token_headers(lock, DAV_HEADER_IF);\n\n \tstrbuf_addf(&out_buffer.buf, \"%s\\n\", sha1_to_hex(sha1));\n\n@@ -1756,7 +1772,6 @@ static int update_remote(unsigned char *sha1,\nstruct remote_lock *lock)\n \tif (start_active_slot(slot)) {\n \t\trun_active_slot(slot);\n \t\tstrbuf_release(&out_buffer.buf);\n-\t\tfree(if_header);\n \t\tif (results.curl_result != CURLE_OK) {\n \t\t\tfprintf(stderr,\n \t\t\t\t\"PUT error: curl result=%d, HTTP code=%ld\\n\",\n@@ -1766,7 +1781,6 @@ static int update_remote(unsigned char *sha1,\nstruct remote_lock *lock)\n \t\t}\n \t} else {\n \t\tstrbuf_release(&out_buffer.buf);\n-\t\tfree(if_header);\n \t\tfprintf(stderr, \"Unable to start PUT request\\n\");\n \t\treturn 0;\n \t}\n@@ -1948,15 +1962,12 @@ static void update_remote_info_refs(struct\nremote_lock *lock)\n \tstruct buffer buffer = { STRBUF_INIT, 0 };\n \tstruct active_request_slot *slot;\n \tstruct slot_results results;\n-\tchar *if_header;\n-\tstruct curl_slist *dav_headers = NULL;\n+\tstruct curl_slist *dav_headers;\n\n \tremote_ls(\"refs/\", (PROCESS_FILES | RECURSIVE),\n \t\t  add_remote_info_ref, &buffer.buf);\n \tif (!aborted) {\n-\t\tif_header = xmalloc(strlen(lock->token) + 25);\n-\t\tsprintf(if_header, \"If: (<%s>)\", lock->token);\n-\t\tdav_headers = curl_slist_append(dav_headers, if_header);\n+\t\tdav_headers = get_dav_token_headers(lock, DAV_HEADER_IF);\n\n \t\tslot = get_active_slot();\n \t\tslot->results = &results;\n@@ -1978,7 +1989,6 @@ static void update_remote_info_refs(struct\nremote_lock *lock)\n \t\t\t\t\tresults.curl_result, results.http_code);\n \t\t\t}\n \t\t}\n-\t\tfree(if_header);\n \t}\n \tstrbuf_release(&buffer.buf);\n }\n-- \n1.6.0.4\n"},{"id":"101727","messageId":"alpine.DEB.1.00.0901240507070.13232@racer","threadId":"17335","inReplyTo":"be6fef0d0901231800t6943b01dwbca976d5e9f3397@mail.gmail.com","subject":"Re: [PATCH] http-push: refactor lock-related headers creation for curl requests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-24T04:11:21Z","receivedAt":"2009-01-24T04:11:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 24 Jan 2009, Ray Chuan wrote:\n\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nMake that an Acked-by:\n\n> +\tif(options & DAV_HEADER_IF) {\n> +\t\tstrbuf_addf(&buf, \"If: (<%s>)\", lock->token);\n> +\t\tdav_headers = curl_slist_append(dav_headers, buf.buf);\n> +\t\tstrbuf_reset(&buf);\n\nBTW in case anyone is puzzled (like I was): curl_slist_append() takes a \n\"char *\" as second parameter, but does not take custody of the buffer; \ninstead, it strdup()s it.  See\nhttp://cool.haxx.se/cvs.cgi/curl/lib/sendf.c?rev=1.155&content-type=text/vnd.viewcvs-markup\nfor details.\n\nBTNW this should be mentioned in the commit message, too, to spare other \npeople the puzzlement.\n\nCiao,\nDscho\n"},{"id":"101729","messageId":"7vy6x1nuxt.fsf@gitster.siamese.dyndns.org","threadId":"17335","inReplyTo":"alpine.DEB.1.00.0901240507070.13232@racer","subject":"Re: [PATCH] http-push: refactor lock-related headers creation for curl requests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-24T05:18:22Z","receivedAt":"2009-01-24T05:18:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Sat, 24 Jan 2009, Ray Chuan wrote:\n>\n>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> Make that an Acked-by:\n>\n>> +\tif(options & DAV_HEADER_IF) {\n>> +\t\tstrbuf_addf(&buf, \"If: (<%s>)\", lock->token);\n>> +\t\tdav_headers = curl_slist_append(dav_headers, buf.buf);\n>> +\t\tstrbuf_reset(&buf);\n>\n> BTW in case anyone is puzzled (like I was): curl_slist_append() takes a \n> \"char *\" as second parameter, but does not take custody of the buffer; \n> instead, it strdup()s it.  See\n> http://cool.haxx.se/cvs.cgi/curl/lib/sendf.c?rev=1.155&content-type=text/vnd.viewcvs-markup\n> for details.\n>\n> BTNW this should be mentioned in the commit message, too, to spare other \n> people the puzzlement.\n\nYeah, but your advice is too late --- I already lost a few minutes solving\nmy puzzlement with manpages.\n\nThanks, both.\n"}]}