{"thread":{"id":"17226","subject":"[PATCH] http-push: fix off-by-path_len","startedAt":"2009-01-17T15:36:26Z","lastAt":"2009-01-18T21:10:44Z","messageCount":33,"participants":["Johannes Schindelin","Mike Hommey","Junio C Hamano","Boyd Stephen Smith Jr."],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"100837","messageId":"alpine.DEB.1.00.0901171632330.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":null,"subject":"[PATCH] http-push: fix off-by-path_len","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-17T15:36:26Z","receivedAt":"2009-01-17T15:36:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen getting the result of remote_ls(), we were advancing the variable\n\"path\" to the relative path inside the repository.\n\nHowever, then we went on to malloc a bogus amount of memory: we were\nsubtracting the prefix length _again_, quite possibly getting something\nnegative, which xmalloc() interprets as really, really much.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tNote that the push in t5540 is still broken, as http-push does\n\tnot handle packed-refs (when looking what branches are on the \n\tremote side).\n\n\tIt should not even try to access the directory structure under\n\trefs/ to begin with, but read info/refs instead.\n\n\tHowever, that is just one example of the ugliness that is \n\thttp-push.c; it also seems to be a perfect example of a copy-pasting \n\thell; just look at the output of \"git grep\n\tcurl_easy_setopt http-push.c\".\n\n\tThere _has_ to be lot of room for improvement.\n\n http-push.c |   10 +++++++---\n 1 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 9fcccee..cb5bf95 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1435,10 +1435,8 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n \t\t\t}\n \t\t\tif (path) {\n \t\t\t\tpath += remote->path_len;\n+\t\t\t\tls->dentry_name = xstrdup(path);\n \t\t\t}\n-\t\t\tls->dentry_name = xmalloc(strlen(path) -\n-\t\t\t\t\t\t  remote->path_len + 1);\n-\t\t\tstrcpy(ls->dentry_name, path + remote->path_len);\n \t\t} else if (!strcmp(ctx->name, DAV_PROPFIND_COLLECTION)) {\n \t\t\tls->dentry_flags |= IS_DIR;\n \t\t}\n@@ -1449,6 +1447,12 @@ static void handle_remote_ls_ctx(struct xml_ctx *ctx, int tag_closed)\n \t}\n }\n \n+/*\n+ * NEEDSWORK: remote_ls() ignores info/refs on the remote side.  But it\n+ * should _only_ heed the information from that file, instead of trying to\n+ * determine the refs from the remote file system (badly: it does not even\n+ * know about packed-refs).\n+ */\n static void remote_ls(const char *path, int flags,\n \t\t      void (*userFunc)(struct remote_ls_ctx *ls),\n \t\t      void *userData)\n-- \n1.6.1.325.g062d4\n"},{"id":"100840","messageId":"alpine.DEB.1.00.0901171639450.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"alpine.DEB.1.00.0901171632330.3586@pacific.mpi-cbg.de","subject":"Where's Nick?, was Re: [PATCH] http-push: fix off-by-path_len","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-17T15:40:24Z","receivedAt":"2009-01-17T15:40:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nNick's email address seems to be no longer functional.  Any idea where \nhe's hanging about these days?\n\nCiao,\nDscho\n"},{"id":"100841","messageId":"alpine.DEB.1.00.0901171640530.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"alpine.DEB.1.00.0901171632330.3586@pacific.mpi-cbg.de","subject":"[PATCH] t5540: clarify that http-push does not handle packed-refs on the remote","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-17T15:41:41Z","receivedAt":"2009-01-17T15:41:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tOn Sat, 17 Jan 2009, Johannes Schindelin wrote:\n\n\t> \tNote that the push in t5540 is still broken, as http-push does\n\t> \tnot handle packed-refs (when looking what branches are on the \n\t> \tremote side).\n\n\t... and this clarifies the issue.\n\n t/t5540-http-push.sh |   18 +++++++++++++++---\n 1 files changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh\nindex 22cfbb6..c236b5e 100755\n--- a/t/t5540-http-push.sh\n+++ b/t/t5540-http-push.sh\n@@ -51,17 +51,29 @@ test_expect_success 'clone remote repository' '\n \tgit clone $HTTPD_URL/test_repo.git test_repo_clone\n '\n \n-test_expect_failure 'push to remote repository' '\n+test_expect_failure 'push to remote repository with packed refs' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \t: >path2 &&\n \tgit add path2 &&\n \ttest_tick &&\n \tgit commit -m path2 &&\n+\tHEAD=$(git rev-parse --verify HEAD) &&\n \tgit push &&\n-\t[ -f \"$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git/refs/heads/master\" ]\n+\t(cd \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git &&\n+\t test $HEAD = $(git rev-parse --verify HEAD))\n '\n \n-test_expect_failure 'create and delete remote branch' '\n+test_expect_success ' push to remote repository with unpacked refs' '\n+\t(cd \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git &&\n+\t rm packed-refs &&\n+\t git update-ref refs/heads/master \\\n+\t\t0c973ae9bd51902a28466f3850b543fa66a6aaf4) &&\n+\tgit push &&\n+\t(cd \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git &&\n+\t test $HEAD = $(git rev-parse --verify HEAD))\n+'\n+\n+test_expect_success 'create and delete remote branch' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \tgit checkout -b dev &&\n \t: >path3 &&\n-- \n1.6.1.325.g062d4\n"},{"id":"100925","messageId":"20090118074911.GB30228@glandium.org","threadId":"17226","inReplyTo":"alpine.DEB.1.00.0901171632330.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] http-push: fix off-by-path_len","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T07:49:11Z","receivedAt":"2009-01-18T07:49:11Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sat, Jan 17, 2009 at 04:36:26PM +0100, Johannes Schindelin wrote:\n> \n> When getting the result of remote_ls(), we were advancing the variable\n> \"path\" to the relative path inside the repository.\n> \n> However, then we went on to malloc a bogus amount of memory: we were\n> subtracting the prefix length _again_, quite possibly getting something\n> negative, which xmalloc() interprets as really, really much.\n> \n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> \n> \tNote that the push in t5540 is still broken, as http-push does\n> \tnot handle packed-refs (when looking what branches are on the \n> \tremote side).\n> \n> \tIt should not even try to access the directory structure under\n> \trefs/ to begin with, but read info/refs instead.\n\nIt would actually need to do both, because nothing guarantees info/refs\nis up-to-date.\n\n> \tHowever, that is just one example of the ugliness that is \n> \thttp-push.c; it also seems to be a perfect example of a copy-pasting \n> \thell; just look at the output of \"git grep\n> \tcurl_easy_setopt http-push.c\".\n\nLikewise for http.c.\n\n> \tThere _has_ to be lot of room for improvement.\n\nAnd I realize I have had a partial improvement on that sitting on my\nharddrive, without me having time (nor motivation) to go further.\n\nMaybe it's time I let it go and post the work in progress for someone\nelse to take over.\n\nMike\n"},{"id":"100930","messageId":"1232265877-3649-1-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"20090118074911.GB30228@glandium.org","subject":"[WIP Patch 00/12] Refactoring the http API","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:25Z","receivedAt":"2009-01-18T08:04:25Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"> And I realize I have had a partial improvement on that sitting on my\n> harddrive, without me having time (nor motivation) to go further.\n>\n> Maybe it's time I let it go and post the work in progress for someone\n> else to take over.\n\nHere it is. \n\nNote I already sent the first patch a year ago:\n http://markmail.org/message/s4w4pmla4tzjkpsf\n\nThis set of patches only deals with HTTP GET requests, so all of http-push\nis not taken care of.\n\nAs it is work in progress, some error handling might have regressions, but\nthe original error handling is not necessarily much better.\n\nAlso note I only rebased my one-year-old work on current master, and haven't\nactually tested it, though, as the code hasn't changed much, I guess it\nshould be fine.\n\nI hope someone will have an itch to scratch to improve the whole thing.\n\nMike Hommey (12):\n  Don't expect verify_pack() callers to set pack_size\n  Some cleanup in get_refs_via_curl()\n  Two new functions for the http API\n  Use the new http API in http_fetch_ref()\n  Use the new http API in get_refs_via_curl()\n  Use the new http API in http-walker.c:fetch_indices()\n  Use the new http API in http-push.c:fetch_indices()\n  Use the new http API in update_remote_info_refs()\n  Use the new http API in fetch_symref()\n  Use the new http API in http-walker.c:fetch_index()\n  Use the new http API in http-push.c:fetch_index()\n  Use the new http API in http-walker.c:fetch_pack()\n\n http-push.c   |  159 ++++++++++-----------------------------------------------\n http-walker.c |  141 ++++++--------------------------------------------\n http.c        |  112 ++++++++++++++++++++++++++++++++--------\n http.h        |   17 ++++++\n pack-check.c  |    8 ++-\n transport.c   |   29 ++--------\n 6 files changed, 162 insertions(+), 304 deletions(-)\n"},{"id":"100927","messageId":"1232265877-3649-2-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-1-git-send-email-mh@glandium.org","subject":"[WIP Patch 01/12] Don't expect verify_pack() callers to set pack_size","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:26Z","receivedAt":"2009-01-18T08:04:26Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"Since use_pack() will end up populating pack_size if it is not already set,\nwe can just adapt the code in verify_packfile() such that it doesn't require\npack_size to be set beforehand.\n\nThis allows callers not to have to set pack_size themselves, and we can thus\nrevert changes from 1c23d794 (Don't die in git-http-fetch when fetching packs).\n\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-push.c   |    3 ---\n http-walker.c |    1 -\n pack-check.c  |    8 +++++---\n 3 files changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex a4b7d08..e69179b 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -771,14 +771,11 @@ static void finish_request(struct transfer_request *request)\n \t\t\t\trequest->url, curl_errorstr);\n \t\t\tremote->can_update_info_refs = 0;\n \t\t} else {\n-\t\t\toff_t pack_size = ftell(request->local_stream);\n-\n \t\t\tfclose(request->local_stream);\n \t\t\trequest->local_stream = NULL;\n \t\t\tif (!move_temp_to_file(request->tmpfile,\n \t\t\t\t\t       request->filename)) {\n \t\t\t\ttarget = (struct packed_git *)request->userData;\n-\t\t\t\ttarget->pack_size = pack_size;\n \t\t\t\tlst = &remote->packs;\n \t\t\t\twhile (*lst != target)\n \t\t\t\t\tlst = &((*lst)->next);\ndiff --git a/http-walker.c b/http-walker.c\nindex 7271c7d..0139d1e 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -785,7 +785,6 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha\n \t\treturn error(\"Unable to start request\");\n \t}\n \n-\ttarget->pack_size = ftell(packfile);\n \tfclose(packfile);\n \n \tret = move_temp_to_file(tmpfile, filename);\ndiff --git a/pack-check.c b/pack-check.c\nindex 90c33b1..166ca70 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -49,7 +49,7 @@ static int verify_packfile(struct packed_git *p,\n \tconst unsigned char *index_base = p->index_data;\n \tgit_SHA_CTX ctx;\n \tunsigned char sha1[20], *pack_sig;\n-\toff_t offset = 0, pack_sig_ofs = p->pack_size - 20;\n+\toff_t offset = 0, pack_sig_ofs = 0;\n \tuint32_t nr_objects, i;\n \tint err = 0;\n \tstruct idx_entry *entries;\n@@ -61,14 +61,16 @@ static int verify_packfile(struct packed_git *p,\n \t */\n \n \tgit_SHA1_Init(&ctx);\n-\twhile (offset < pack_sig_ofs) {\n+\tdo {\n \t\tunsigned int remaining;\n \t\tunsigned char *in = use_pack(p, w_curs, offset, &remaining);\n \t\toffset += remaining;\n+\t\tif (!pack_sig_ofs)\n+\t\t\tpack_sig_ofs = p->pack_size - 20;\n \t\tif (offset > pack_sig_ofs)\n \t\t\tremaining -= (unsigned int)(offset - pack_sig_ofs);\n \t\tgit_SHA1_Update(&ctx, in, remaining);\n-\t}\n+\t} while (offset < pack_sig_ofs);\n \tgit_SHA1_Final(sha1, &ctx);\n \tpack_sig = use_pack(p, w_curs, pack_sig_ofs, NULL);\n \tif (hashcmp(sha1, pack_sig))\n-- \n1.6.1.141.gb32a\n"},{"id":"100926","messageId":"1232265877-3649-3-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-2-git-send-email-mh@glandium.org","subject":"[WIP Patch 02/12] Some cleanup in get_refs_via_curl()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:27Z","receivedAt":"2009-01-18T08:04:27Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n transport.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 56831c5..6919ff1 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n \t\tfree(ref);\n \t}\n \n+\thttp_cleanup();\n+\tfree(refs_url);\n \treturn refs;\n }\n \n-- \n1.6.1.141.gb32a\n"},{"id":"100928","messageId":"1232265877-3649-4-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-3-git-send-email-mh@glandium.org","subject":"[WIP Patch 03/12] Two new functions for the http API","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:28Z","receivedAt":"2009-01-18T08:04:28Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"http_get_strbuf and http_get_file allow respectively to retrieve contents of\nan URL to a strbuf or an opened file handle.\n\nBoth these functions are the beginning of the http code refactoring.\n\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http.c |   85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n http.h |   17 +++++++++++++\n 2 files changed, 102 insertions(+), 0 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex ee58799..82534cf 100644\n--- a/http.c\n+++ b/http.c\n@@ -638,3 +638,88 @@ int http_fetch_ref(const char *base, struct ref *ref)\n \tfree(url);\n \treturn ret;\n }\n+\n+/* http_request() targets */\n+#define HTTP_REQUEST_STRBUF\t0\n+#define HTTP_REQUEST_FILE\t1\n+\n+static int http_request(const char *url, void *result, int target, int options)\n+{\n+\tstruct active_request_slot *slot;\n+\tstruct slot_results results;\n+\tstruct curl_slist *headers = NULL;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tslot = get_active_slot();\n+\tslot->results = &results;\n+\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n+\n+\tif (result == NULL) {\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);\n+\t} else {\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, result);\n+\n+\t\tif (target == HTTP_REQUEST_FILE) {\n+\t\t\tlong posn = ftell(result);\n+\t\t\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,\n+\t\t\t\t\t fwrite);\n+\t\t\tif (posn > 0) {\n+\t\t\t\tstrbuf_addf(&buf, \"Range: bytes=%ld-\", posn);\n+\t\t\t\theaders = curl_slist_append(headers, buf.buf);\n+\t\t\t\tstrbuf_reset(&buf);\n+\t\t\t}\n+\t\t\tslot->local = result;\n+\t\t} else\n+\t\t\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,\n+\t\t\t\t\t fwrite_buffer);\n+\t}\n+\n+\tstrbuf_addstr(&buf, \"Pragma:\");\n+\tif (options & HTTP_NO_CACHE)\n+\t\tstrbuf_addstr(&buf, \" no-cache\");\n+\n+\theaders = curl_slist_append(headers, buf.buf);\n+\tstrbuf_release(&buf);\n+\n+\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);\n+\n+\tif (start_active_slot(slot)) {\n+\t\trun_active_slot(slot);\n+\t\tif (results.curl_result == CURLE_OK)\n+\t\t\treturn HTTP_OK;\n+\t\telse if (missing_target(&results))\n+\t\t\treturn HTTP_MISSING_TARGET;\n+\t}\n+\treturn HTTP_ERROR;\n+}\n+\n+int http_get_strbuf(const char *url, struct strbuf *result, int options)\n+{\n+\treturn http_request(url, result, HTTP_REQUEST_STRBUF, options);\n+}\n+\n+int http_get_file(const char *url, const char *filename, int options)\n+{\n+\tint ret;\n+\tstruct strbuf tmpfile = STRBUF_INIT;\n+\tFILE *result;\n+\n+\tstrbuf_addf(&tmpfile, \"%s.temp\", filename);\n+\tresult = fopen(tmpfile.buf, \"a\");\n+\tif (! result) {\n+\t\terror(\"Unable to open local file %s\", tmpfile.buf);\n+\t\tret = HTTP_ERROR;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tret = http_request(url, result, HTTP_REQUEST_FILE, options);\n+\tfclose(result);\n+\n+\tif ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))\n+\t\tret = HTTP_ERROR;\n+cleanup:\n+\tstrbuf_release(&tmpfile);\n+\treturn ret;\n+}\ndiff --git a/http.h b/http.h\nindex 905b462..323c780 100644\n--- a/http.h\n+++ b/http.h\n@@ -104,4 +104,21 @@ static inline int missing__target(int code, int result)\n \n extern int http_fetch_ref(const char *base, struct ref *ref);\n \n+/* Options for http_request_*() */\n+#define HTTP_NO_CACHE\t\t1\n+\n+/* Return values for http_request_*() */\n+#define HTTP_OK\t\t\t0\n+#define HTTP_MISSING_TARGET\t1\n+#define HTTP_ERROR\t\t2\n+\n+/* Requests an url and stores the result in a strbuf.\n+ * If the result pointer is NULL, a HTTP HEAD request is made instead of GET. */\n+int http_get_strbuf(const char *url, struct strbuf *result, int options);\n+\n+/* Downloads an url and stores the result in the given file.\n+ * If a previous interrupted download is detected (i.e. a previous temporary\n+ * file is still around) the download is resumed. */\n+int http_get_file(const char *url, const char *filename, int options);\n+\n #endif /* HTTP_H */\n-- \n1.6.1.141.gb32a\n"},{"id":"100931","messageId":"1232265877-3649-5-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-4-git-send-email-mh@glandium.org","subject":"[WIP Patch 04/12] Use the new http API in http_fetch_ref()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:29Z","receivedAt":"2009-01-18T08:04:29Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http.c |   33 ++++++++-------------------------\n 1 files changed, 8 insertions(+), 25 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 82534cf..0c9504b 100644\n--- a/http.c\n+++ b/http.c\n@@ -604,34 +604,17 @@ int http_fetch_ref(const char *base, struct ref *ref)\n {\n \tchar *url;\n \tstruct strbuf buffer = STRBUF_INIT;\n-\tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n-\tint ret;\n+\tint ret = -1;\n \n \turl = quote_ref_url(base, ref->name);\n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.curl_result == CURLE_OK) {\n-\t\t\tstrbuf_rtrim(&buffer);\n-\t\t\tif (buffer.len == 40)\n-\t\t\t\tret = get_sha1_hex(buffer.buf, ref->old_sha1);\n-\t\t\telse if (!prefixcmp(buffer.buf, \"ref: \")) {\n-\t\t\t\tref->symref = xstrdup(buffer.buf + 5);\n-\t\t\t\tret = 0;\n-\t\t\t} else\n-\t\t\t\tret = 1;\n-\t\t} else {\n-\t\t\tret = error(\"Couldn't get %s for %s\\n%s\",\n-\t\t\t\t    url, ref->name, curl_errorstr);\n+\tif (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) {\n+\t\tstrbuf_rtrim(&buffer);\n+\t\tif (buffer.len == 40)\n+\t\t\tret = get_sha1_hex(buffer.buf, ref->old_sha1);\n+\t\telse if (!prefixcmp(buffer.buf, \"ref: \")) {\n+\t\t\tref->symref = xstrdup(buffer.buf + 5);\n+\t\t\tret = 0;\n \t\t}\n-\t} else {\n-\t\tret = error(\"Unable to start request\");\n \t}\n \n \tstrbuf_release(&buffer);\n-- \n1.6.1.141.gb32a\n"},{"id":"100933","messageId":"1232265877-3649-6-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-5-git-send-email-mh@glandium.org","subject":"[WIP Patch 05/12] Use the new http API in get_refs_via_curl()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:30Z","receivedAt":"2009-01-18T08:04:30Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n transport.c |   27 ++++-----------------------\n 1 files changed, 4 insertions(+), 23 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 6919ff1..55bf274 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -432,9 +432,6 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n \tchar *refs_url;\n \tint i = 0;\n \n-\tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n-\n \tstruct ref *refs = NULL;\n \tstruct ref *ref = NULL;\n \tstruct ref *last_ref = NULL;\n@@ -450,26 +447,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n \trefs_url = xmalloc(strlen(transport->url) + 11);\n \tsprintf(refs_url, \"%s/info/refs\", transport->url);\n \n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, refs_url);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);\n-\n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.curl_result != CURLE_OK) {\n-\t\t\tstrbuf_release(&buffer);\n-\t\t\tif (missing_target(&results))\n-\t\t\t\tdie(\"%s not found: did you run git update-server-info on the server?\", refs_url);\n-\t\t\telse\n-\t\t\t\tdie(\"%s download error - %s\", refs_url, curl_errorstr);\n-\t\t}\n-\t} else {\n-\t\tstrbuf_release(&buffer);\n-\t\tdie(\"Unable to start HTTP request\");\n-\t}\n+\tif (http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE) != HTTP_OK)\n+\t\tgoto cleanup;\n \n \tdata = buffer.buf;\n \tstart = NULL;\n@@ -508,6 +487,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n \t\tfree(ref);\n \t}\n \n+cleanup:\n+\tstrbuf_release(&buffer);\n \thttp_cleanup();\n \tfree(refs_url);\n \treturn refs;\n-- \n1.6.1.141.gb32a\n"},{"id":"100929","messageId":"1232265877-3649-7-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-6-git-send-email-mh@glandium.org","subject":"[WIP Patch 06/12] Use the new http API in http-walker.c:fetch_indices()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:31Z","receivedAt":"2009-01-18T08:04:31Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-walker.c |   31 ++++++++-----------------------\n 1 files changed, 8 insertions(+), 23 deletions(-)\n\ndiff --git a/http-walker.c b/http-walker.c\nindex 0139d1e..edcb779 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -648,9 +648,6 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)\n \tint i = 0;\n \tint ret = 0;\n \n-\tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n-\n \tif (repo->got_indices)\n \t\treturn 0;\n \n@@ -660,27 +657,15 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo)\n \turl = xmalloc(strlen(repo->base) + 21);\n \tsprintf(url, \"%s/objects/info/packs\", repo->base);\n \n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);\n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.curl_result != CURLE_OK) {\n-\t\t\tif (missing_target(&results)) {\n-\t\t\t\trepo->got_indices = 1;\n-\t\t\t\tgoto cleanup;\n-\t\t\t} else {\n-\t\t\t\trepo->got_indices = 0;\n-\t\t\t\tret = error(\"%s\", curl_errorstr);\n-\t\t\t\tgoto cleanup;\n-\t\t\t}\n-\t\t}\n-\t} else {\n+\tswitch (http_get_strbuf(url, &buffer, HTTP_NO_CACHE)) {\n+\tcase HTTP_OK:\n+\t\tbreak;\n+\tcase HTTP_MISSING_TARGET:\n+\t\trepo->got_indices = 1;\n+\t\tgoto cleanup;\n+\tdefault:\n \t\trepo->got_indices = 0;\n-\t\tret = error(\"Unable to start request\");\n+\t\tret = -1;\n \t\tgoto cleanup;\n \t}\n \n-- \n1.6.1.141.gb32a\n"},{"id":"100932","messageId":"1232265877-3649-8-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-7-git-send-email-mh@glandium.org","subject":"[WIP Patch 07/12] Use the new http API in http-push.c:fetch_indices()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:32Z","receivedAt":"2009-01-18T08:04:32Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-push.c |   32 +++++++-------------------------\n 1 files changed, 7 insertions(+), 25 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex e69179b..e0b4f5a 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1008,9 +1008,7 @@ static int fetch_indices(void)\n \tstruct strbuf buffer = STRBUF_INIT;\n \tchar *data;\n \tint i = 0;\n-\n-\tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n+\tint ret = 0;\n \n \tif (push_verbosely)\n \t\tfprintf(stderr, \"Getting pack list\\n\");\n@@ -1018,28 +1016,10 @@ static int fetch_indices(void)\n \turl = xmalloc(strlen(remote->url) + 20);\n \tsprintf(url, \"%sobjects/info/packs\", remote->url);\n \n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);\n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.curl_result != CURLE_OK) {\n-\t\t\tstrbuf_release(&buffer);\n-\t\t\tfree(url);\n-\t\t\tif (results.http_code == 404)\n-\t\t\t\treturn 0;\n-\t\t\telse\n-\t\t\t\treturn error(\"%s\", curl_errorstr);\n-\t\t}\n-\t} else {\n-\t\tstrbuf_release(&buffer);\n-\t\tfree(url);\n-\t\treturn error(\"Unable to start request\");\n+\tif (http_get_strbuf(url, &buffer, 0) != HTTP_OK) {\n+\t\tret = -1;\n+\t\tgoto cleanup;\n \t}\n-\tfree(url);\n \n \tdata = buffer.buf;\n \twhile (i < buffer.len) {\n@@ -1061,8 +1041,10 @@ static int fetch_indices(void)\n \t\ti++;\n \t}\n \n+cleanup:\n \tstrbuf_release(&buffer);\n-\treturn 0;\n+\tfree(url);\n+\treturn ret;\n }\n \n static void one_remote_object(const char *hex)\n-- \n1.6.1.141.gb32a\n"},{"id":"100935","messageId":"1232265877-3649-9-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-8-git-send-email-mh@glandium.org","subject":"[WIP Patch 08/12] Use the new http API in update_remote_info_refs()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:33Z","receivedAt":"2009-01-18T08:04:33Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-push.c |   29 ++++++++++-------------------\n 1 files changed, 10 insertions(+), 19 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex e0b4f5a..7627860 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1960,29 +1960,20 @@ static void update_remote_info_refs(struct remote_lock *lock)\n static int remote_exists(const char *path)\n {\n \tchar *url = xmalloc(strlen(remote->url) + strlen(path) + 1);\n-\tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n-\tint ret = -1;\n+\tint ret;\n \n \tsprintf(url, \"%s%s\", remote->url, path);\n \n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);\n-\n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.http_code == 404)\n-\t\t\tret = 0;\n-\t\telse if (results.curl_result == CURLE_OK)\n-\t\t\tret = 1;\n-\t\telse\n-\t\t\tfprintf(stderr, \"HEAD HTTP error %ld\\n\", results.http_code);\n-\t} else {\n-\t\tfprintf(stderr, \"Unable to start HEAD request\\n\");\n+\tswitch (http_get_strbuf(url, NULL, 0)) {\n+\tcase HTTP_OK:\n+\t\tret = 1;\n+\t\tbreak;\n+\tcase HTTP_MISSING_TARGET:\n+\t\tret = 0;\n+\t\tbreak;\n+\tdefault:\n+\t\tret = -1;\n \t}\n-\n \tfree(url);\n \treturn ret;\n }\n-- \n1.6.1.141.gb32a\n"},{"id":"100936","messageId":"1232265877-3649-10-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-9-git-send-email-mh@glandium.org","subject":"[WIP Patch 09/12] Use the new http API in fetch_symref()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:34Z","receivedAt":"2009-01-18T08:04:34Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-push.c |   19 ++-----------------\n 1 files changed, 2 insertions(+), 17 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 7627860..b32def4 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1982,27 +1982,12 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)\n {\n \tchar *url;\n \tstruct strbuf buffer = STRBUF_INIT;\n-\tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n \n \turl = xmalloc(strlen(remote->url) + strlen(path) + 1);\n \tsprintf(url, \"%s%s\", remote->url, path);\n \n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.curl_result != CURLE_OK) {\n-\t\t\tdie(\"Couldn't get %s for remote symref\\n%s\",\n-\t\t\t    url, curl_errorstr);\n-\t\t}\n-\t} else {\n-\t\tdie(\"Unable to start remote symref request\");\n-\t}\n+\tif (http_get_strbuf(url, &buffer, 0) != HTTP_OK)\n+\t\tdie(\"Couldn't get %s for remote symref\\n\", url);\n \tfree(url);\n \n \tfree(*symref);\n-- \n1.6.1.141.gb32a\n"},{"id":"100934","messageId":"1232265877-3649-11-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-10-git-send-email-mh@glandium.org","subject":"[WIP Patch 10/12] Use the new http API in http-walker.c:fetch_index()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:35Z","receivedAt":"2009-01-18T08:04:35Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-walker.c |   55 +++++--------------------------------------------------\n 1 files changed, 5 insertions(+), 50 deletions(-)\n\ndiff --git a/http-walker.c b/http-walker.c\nindex edcb779..bc1db2b 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -368,15 +368,7 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch\n \tchar *hex = sha1_to_hex(sha1);\n \tchar *filename;\n \tchar *url;\n-\tchar tmpfile[PATH_MAX];\n-\tlong prev_posn = 0;\n-\tchar range[RANGE_HEADER_SIZE];\n-\tstruct curl_slist *range_header = NULL;\n-\tstruct walker_data *data = walker->data;\n-\n-\tFILE *indexfile;\n-\tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n+\tint ret = 0;\n \n \tif (has_pack_index(sha1))\n \t\treturn 0;\n@@ -388,48 +380,11 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch\n \tsprintf(url, \"%s/objects/pack/pack-%s.idx\", repo->base, hex);\n \n \tfilename = sha1_pack_index_name(sha1);\n-\tsnprintf(tmpfile, sizeof(tmpfile), \"%s.temp\", filename);\n-\tindexfile = fopen(tmpfile, \"a\");\n-\tif (!indexfile)\n-\t\treturn error(\"Unable to open local file %s for pack index\",\n-\t\t\t     tmpfile);\n-\n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, indexfile);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, data->no_pragma_header);\n-\tslot->local = indexfile;\n-\n-\t/* If there is data present from a previous transfer attempt,\n-\t   resume where it left off */\n-\tprev_posn = ftell(indexfile);\n-\tif (prev_posn>0) {\n-\t\tif (walker->get_verbosely)\n-\t\t\tfprintf(stderr,\n-\t\t\t\t\"Resuming fetch of index for pack %s at byte %ld\\n\",\n-\t\t\t\thex, prev_posn);\n-\t\tsprintf(range, \"Range: bytes=%ld-\", prev_posn);\n-\t\trange_header = curl_slist_append(range_header, range);\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, range_header);\n-\t}\n-\n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.curl_result != CURLE_OK) {\n-\t\t\tfclose(indexfile);\n-\t\t\treturn error(\"Unable to get pack index %s\\n%s\", url,\n-\t\t\t\t     curl_errorstr);\n-\t\t}\n-\t} else {\n-\t\tfclose(indexfile);\n-\t\treturn error(\"Unable to start request\");\n-\t}\n-\n-\tfclose(indexfile);\n \n-\treturn move_temp_to_file(tmpfile, filename);\n+\tif (http_get_file(url, filename, 0) != HTTP_OK)\n+\t\tret = error(\"Unable to get pack index %s\\n\", url);\n+\tfree(url);\n+\treturn ret;\n }\n \n static int setup_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1)\n-- \n1.6.1.141.gb32a\n"},{"id":"100937","messageId":"1232265877-3649-12-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-11-git-send-email-mh@glandium.org","subject":"[WIP Patch 11/12] Use the new http API in http-push.c:fetch_index()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:36Z","receivedAt":"2009-01-18T08:04:36Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-push.c |   76 ++++++----------------------------------------------------\n 1 files changed, 8 insertions(+), 68 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex b32def4..0812f58 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -898,32 +898,15 @@ static int fetch_index(unsigned char *sha1)\n \tchar *hex = sha1_to_hex(sha1);\n \tchar *filename;\n \tchar *url;\n-\tchar tmpfile[PATH_MAX];\n-\tlong prev_posn = 0;\n-\tchar range[RANGE_HEADER_SIZE];\n-\tstruct curl_slist *range_header = NULL;\n-\n-\tFILE *indexfile;\n-\tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n+\tint ret = 0;\n \n \t/* Don't use the index if the pack isn't there */\n \turl = xmalloc(strlen(remote->url) + 64);\n \tsprintf(url, \"%sobjects/pack/pack-%s.pack\", remote->url, hex);\n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);\n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.curl_result != CURLE_OK) {\n-\t\t\tfree(url);\n-\t\t\treturn error(\"Unable to verify pack %s is available\",\n-\t\t\t\t     hex);\n-\t\t}\n-\t} else {\n+\tif (http_get_strbuf(url, NULL, 0)) {\n \t\tfree(url);\n-\t\treturn error(\"Unable to start request\");\n+\t\treturn error(\"Unable to verify pack %s is available\",\n+\t\t\t     hex);\n \t}\n \n \tif (has_pack_index(sha1)) {\n@@ -937,55 +920,12 @@ static int fetch_index(unsigned char *sha1)\n \tsprintf(url, \"%sobjects/pack/pack-%s.idx\", remote->url, hex);\n \n \tfilename = sha1_pack_index_name(sha1);\n-\tsnprintf(tmpfile, sizeof(tmpfile), \"%s.temp\", filename);\n-\tindexfile = fopen(tmpfile, \"a\");\n-\tif (!indexfile) {\n-\t\tfree(url);\n-\t\treturn error(\"Unable to open local file %s for pack index\",\n-\t\t\t     tmpfile);\n-\t}\n-\n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, indexfile);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header);\n-\tslot->local = indexfile;\n-\n-\t/* If there is data present from a previous transfer attempt,\n-\t   resume where it left off */\n-\tprev_posn = ftell(indexfile);\n-\tif (prev_posn>0) {\n-\t\tif (push_verbosely)\n-\t\t\tfprintf(stderr,\n-\t\t\t\t\"Resuming fetch of index for pack %s at byte %ld\\n\",\n-\t\t\t\thex, prev_posn);\n-\t\tsprintf(range, \"Range: bytes=%ld-\", prev_posn);\n-\t\trange_header = curl_slist_append(range_header, range);\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, range_header);\n-\t}\n-\n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.curl_result != CURLE_OK) {\n-\t\t\tfree(url);\n-\t\t\tfclose(indexfile);\n-\t\t\treturn error(\"Unable to get pack index %s\\n%s\", url,\n-\t\t\t\t     curl_errorstr);\n-\t\t}\n-\t} else {\n-\t\tfree(url);\n-\t\tfclose(indexfile);\n-\t\treturn error(\"Unable to start request\");\n-\t}\n \n+\tif (http_get_file(url, filename, 0) != HTTP_OK)\n+\t\tret = error(\"Unable to get pack index %s\\n\", url);\n+\tfree(filename);\n \tfree(url);\n-\tfclose(indexfile);\n-\n-\treturn move_temp_to_file(tmpfile, filename);\n+\treturn ret;\n }\n \n static int setup_index(unsigned char *sha1)\n-- \n1.6.1.141.gb32a\n"},{"id":"100938","messageId":"1232265877-3649-13-git-send-email-mh@glandium.org","threadId":"17226","inReplyTo":"1232265877-3649-12-git-send-email-mh@glandium.org","subject":"[WIP Patch 12/12] Use the new http API in http-walker.c:fetch_pack()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T08:04:37Z","receivedAt":"2009-01-18T08:04:37Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http-walker.c |   54 ++++--------------------------------------------------\n 1 files changed, 4 insertions(+), 50 deletions(-)\n\ndiff --git a/http-walker.c b/http-walker.c\nindex bc1db2b..c201463 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -656,17 +656,8 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha\n \tchar *url;\n \tstruct packed_git *target;\n \tstruct packed_git **lst;\n-\tFILE *packfile;\n \tchar *filename;\n-\tchar tmpfile[PATH_MAX];\n-\tint ret;\n-\tlong prev_posn = 0;\n-\tchar range[RANGE_HEADER_SIZE];\n-\tstruct curl_slist *range_header = NULL;\n-\tstruct walker_data *data = walker->data;\n-\n-\tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n+\tint ret = 0;\n \n \tif (fetch_indices(walker, repo))\n \t\treturn -1;\n@@ -686,48 +677,11 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha\n \t\trepo->base, sha1_to_hex(target->sha1));\n \n \tfilename = sha1_pack_name(target->sha1);\n-\tsnprintf(tmpfile, sizeof(tmpfile), \"%s.temp\", filename);\n-\tpackfile = fopen(tmpfile, \"a\");\n-\tif (!packfile)\n-\t\treturn error(\"Unable to open local file %s for pack\",\n-\t\t\t     tmpfile);\n-\n-\tslot = get_active_slot();\n-\tslot->results = &results;\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, packfile);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, data->no_pragma_header);\n-\tslot->local = packfile;\n-\n-\t/* If there is data present from a previous transfer attempt,\n-\t   resume where it left off */\n-\tprev_posn = ftell(packfile);\n-\tif (prev_posn>0) {\n-\t\tif (walker->get_verbosely)\n-\t\t\tfprintf(stderr,\n-\t\t\t\t\"Resuming fetch of pack %s at byte %ld\\n\",\n-\t\t\t\tsha1_to_hex(target->sha1), prev_posn);\n-\t\tsprintf(range, \"Range: bytes=%ld-\", prev_posn);\n-\t\trange_header = curl_slist_append(range_header, range);\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, range_header);\n-\t}\n \n-\tif (start_active_slot(slot)) {\n-\t\trun_active_slot(slot);\n-\t\tif (results.curl_result != CURLE_OK) {\n-\t\t\tfclose(packfile);\n-\t\t\treturn error(\"Unable to get pack file %s\\n%s\", url,\n-\t\t\t\t     curl_errorstr);\n-\t\t}\n-\t} else {\n-\t\tfclose(packfile);\n-\t\treturn error(\"Unable to start request\");\n-\t}\n-\n-\tfclose(packfile);\n+\tif (http_get_file(url, filename, 0) != HTTP_OK)\n+\t\tret = error(\"Unable to get pack file %s\\n\", url);\n+\tfree(url);\n \n-\tret = move_temp_to_file(tmpfile, filename);\n \tif (ret)\n \t\treturn ret;\n \n-- \n1.6.1.141.gb32a\n"},{"id":"100939","messageId":"7v8wp9yq23.fsf@gitster.siamese.dyndns.org","threadId":"17226","inReplyTo":"1232265877-3649-1-git-send-email-mh@glandium.org","subject":"Re: [WIP Patch 00/12] Refactoring the http API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-18T08:30:12Z","receivedAt":"2009-01-18T08:30:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey <mh@glandium.org> writes:\n\n> As it is work in progress, some error handling might have regressions, but\n> the original error handling is not necessarily much better.\n>\n> Also note I only rebased my one-year-old work on current master, and haven't\n> actually tested it, though, as the code hasn't changed much, I guess it\n> should be fine.\n> ...\n>  6 files changed, 162 insertions(+), 304 deletions(-)\n\nThanks.\n\nThis looks like a very nice code reduction, and the first few patches\nlooked obviously correct, too ;-)\n\nBut I am puzzled by what you mean by \"haven't actually tested it\".  Do you\nmean you do not use http transport very much yourself, or even when you do\nyou do not use a version of git with these patches applied?\n"},{"id":"100940","messageId":"20090118091219.GA6505@glandium.org","threadId":"17226","inReplyTo":"7v8wp9yq23.fsf@gitster.siamese.dyndns.org","subject":"Re: [WIP Patch 00/12] Refactoring the http API","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T09:12:19Z","receivedAt":"2009-01-18T09:12:19Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Jan 18, 2009 at 12:30:12AM -0800, Junio C Hamano wrote:\n> Mike Hommey <mh@glandium.org> writes:\n> \n> > As it is work in progress, some error handling might have regressions, but\n> > the original error handling is not necessarily much better.\n> >\n> > Also note I only rebased my one-year-old work on current master, and haven't\n> > actually tested it, though, as the code hasn't changed much, I guess it\n> > should be fine.\n> > ...\n> >  6 files changed, 162 insertions(+), 304 deletions(-)\n> \n> Thanks.\n> \n> This looks like a very nice code reduction, and the first few patches\n> looked obviously correct, too ;-)\n> \n> But I am puzzled by what you mean by \"haven't actually tested it\".  Do you\n> mean you do not use http transport very much yourself, or even when you do\n> you do not use a version of git with these patches applied?\n\nI mean I haven't tested the rebased version. The original version was\ntested extensively a year ago. I don't use http transport that much\nnow.\n\nMike\n"},{"id":"100949","messageId":"200901180529.28980.bss@iguanasuicide.net","threadId":"17226","inReplyTo":"20090118091219.GA6505@glandium.org","subject":"Re: [WIP Patch 00/12] Refactoring the http API","fromName":"Boyd Stephen Smith Jr.","fromEmail":"bss@iguanasuicide.net","sentAt":"2009-01-18T11:29:28Z","receivedAt":"2009-01-18T11:29:28Z","isPatch":true,"sender":{"key":"bss@iguanasuicide.net","avatar":"https://gravatar.com/avatar/84b95eeff194b816c1568b1339e63e4b229825298664a9037b9f1ec713ead1e3?d=mp&s=160"},"body":"On Sunday 18 January 2009, Mike Hommey <mh@glandium.org> wrote about 'Re: \n[WIP Patch 00/12] Refactoring the http API':\n>On Sun, Jan 18, 2009 at 12:30:12AM -0800, Junio C Hamano wrote:\n>> Mike Hommey <mh@glandium.org> writes:\n>> > [I]\n>> > haven't actually tested it, though, as the code hasn't changed much,\n>> > I guess it should be fine.\n>> >  6 files changed, 162 insertions(+), 304 deletions(-)\n>>\n>> Thanks.\n>> But I am puzzled by what you mean by \"haven't actually tested it\".  Do\n>> you mean you do not use http transport very much yourself, or even when\n>> you do you do not use a version of git with these patches applied?\n>I mean I haven't tested the rebased version. The original version was\n>tested extensively a year ago. I don't use http transport that much\n>now.\n\nI know it's uncool of me to ask to do this, BUT... Do we have a good \ntest-suite for this?  If so, I can always run it against any number of \ndifferent WEBDAV implementations.  I have a glut of CPU time on many of my \nsystems and enjoy breaking things.\n-- \nBoyd Stephen Smith Jr.                     ,= ,-_-. =. \nbss@iguanasuicide.net                     ((_/)o o(\\_))\nICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' \nhttp://iguanasuicide.net/                      \\_/     \n"},{"id":"100985","messageId":"alpine.DEB.1.00.0901181602580.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"1232265877-3649-4-git-send-email-mh@glandium.org","subject":"Re: [WIP Patch 03/12] Two new functions for the http API","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-18T15:03:20Z","receivedAt":"2009-01-18T15:03:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 18 Jan 2009, Mike Hommey wrote:\n\n> http_get_strbuf and http_get_file allow respectively to retrieve contents of\n> an URL to a strbuf or an opened file handle.\n\nYesss!  Why have you hidden them that long?\n\nCiao,\nDscho\n"},{"id":"100988","messageId":"alpine.DEB.1.00.0901181607210.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"1232265877-3649-5-git-send-email-mh@glandium.org","subject":"Re: [WIP Patch 04/12] Use the new http API in http_fetch_ref()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-18T15:10:38Z","receivedAt":"2009-01-18T15:10:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 18 Jan 2009, Mike Hommey wrote:\n\n> diff --git a/http.c b/http.c\n> index 82534cf..0c9504b 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -604,34 +604,17 @@ int http_fetch_ref(const char *base, struct ref *ref)\n>  {\n>  \tchar *url;\n>  \tstruct strbuf buffer = STRBUF_INIT;\n> -\tstruct active_request_slot *slot;\n> -\tstruct slot_results results;\n> -\tint ret;\n> +\tint ret = -1;\n>  \n>  \turl = quote_ref_url(base, ref->name);\n> -\tslot = get_active_slot();\n> -\tslot->results = &results;\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n> -\tif (start_active_slot(slot)) {\n> -\t\trun_active_slot(slot);\n> -\t\tif (results.curl_result == CURLE_OK) {\n> -\t\t\tstrbuf_rtrim(&buffer);\n> -\t\t\tif (buffer.len == 40)\n> -\t\t\t\tret = get_sha1_hex(buffer.buf, ref->old_sha1);\n> -\t\t\telse if (!prefixcmp(buffer.buf, \"ref: \")) {\n> -\t\t\t\tref->symref = xstrdup(buffer.buf + 5);\n> -\t\t\t\tret = 0;\n> -\t\t\t} else\n> -\t\t\t\tret = 1;\n> -\t\t} else {\n> -\t\t\tret = error(\"Couldn't get %s for %s\\n%s\",\n> -\t\t\t\t    url, ref->name, curl_errorstr);\n> +\tif (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) {\n> +\t\tstrbuf_rtrim(&buffer);\n> +\t\tif (buffer.len == 40)\n> +\t\t\tret = get_sha1_hex(buffer.buf, ref->old_sha1);\n> +\t\telse if (!prefixcmp(buffer.buf, \"ref: \")) {\n> +\t\t\tref->symref = xstrdup(buffer.buf + 5);\n> +\t\t\tret = 0;\n>  \t\t}\n> -\t} else {\n> -\t\tret = error(\"Unable to start request\");\n>  \t}\n\nWhy not keep that error?\n\nBTW I had to scratch my head for a few seconds why you do not need to \nset \"else ret=1;\" for the HTTP_OK case; you set ret = 1 in the beginning.  \nI'd rather put that back for clarity.\n\nCiao,\nDscho\n"},{"id":"100990","messageId":"alpine.DEB.1.00.0901181611380.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"1232265877-3649-6-git-send-email-mh@glandium.org","subject":"Re: [WIP Patch 05/12] Use the new http API in get_refs_via_curl()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-18T15:12:56Z","receivedAt":"2009-01-18T15:12:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 18 Jan 2009, Mike Hommey wrote:\n\n> @@ -450,26 +447,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n>  \trefs_url = xmalloc(strlen(transport->url) + 11);\n>  \tsprintf(refs_url, \"%s/info/refs\", transport->url);\n>  \n> -\tslot = get_active_slot();\n> -\tslot->results = &results;\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_URL, refs_url);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);\n> -\n> -\tif (start_active_slot(slot)) {\n> -\t\trun_active_slot(slot);\n> -\t\tif (results.curl_result != CURLE_OK) {\n> -\t\t\tstrbuf_release(&buffer);\n> -\t\t\tif (missing_target(&results))\n> -\t\t\t\tdie(\"%s not found: did you run git update-server-info on the server?\", refs_url);\n> -\t\t\telse\n> -\t\t\t\tdie(\"%s download error - %s\", refs_url, curl_errorstr);\n> -\t\t}\n> -\t} else {\n> -\t\tstrbuf_release(&buffer);\n> -\t\tdie(\"Unable to start HTTP request\");\n> -\t}\n> +\tif (http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE) != HTTP_OK)\n> +\t\tgoto cleanup;\n\nWhy not die() as the original code?\n\nCiao,\nDscho\n"},{"id":"100991","messageId":"alpine.DEB.1.00.0901181613580.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"1232265877-3649-8-git-send-email-mh@glandium.org","subject":"Re: [WIP Patch 07/12] Use the new http API in http-push.c:fetch_indices()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-18T15:14:35Z","receivedAt":"2009-01-18T15:14:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 18 Jan 2009, Mike Hommey wrote:\n\n> @@ -1018,28 +1016,10 @@ static int fetch_indices(void)\n>  \turl = xmalloc(strlen(remote->url) + 20);\n>  \tsprintf(url, \"%sobjects/info/packs\", remote->url);\n>  \n> -\tslot = get_active_slot();\n> -\tslot->results = &results;\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);\n> -\tif (start_active_slot(slot)) {\n> -\t\trun_active_slot(slot);\n> -\t\tif (results.curl_result != CURLE_OK) {\n> -\t\t\tstrbuf_release(&buffer);\n> -\t\t\tfree(url);\n> -\t\t\tif (results.http_code == 404)\n> -\t\t\t\treturn 0;\n> -\t\t\telse\n> -\t\t\t\treturn error(\"%s\", curl_errorstr);\n> -\t\t}\n> -\t} else {\n> -\t\tstrbuf_release(&buffer);\n> -\t\tfree(url);\n> -\t\treturn error(\"Unable to start request\");\n> +\tif (http_get_strbuf(url, &buffer, 0) != HTTP_OK) {\n> +\t\tret = -1;\n> +\t\tgoto cleanup;\n>  \t}\n\nLet's make that\n\n\t\tret = error(\"%s\", curl_errorstr);\n\nokay?\n\nCiao,\nDscho\n"},{"id":"101001","messageId":"alpine.DEB.1.00.0901181615070.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"1232265877-3649-9-git-send-email-mh@glandium.org","subject":"Re: [WIP Patch 08/12] Use the new http API in update_remote_info_refs()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-18T15:18:16Z","receivedAt":"2009-01-18T15:18:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 18 Jan 2009, Mike Hommey wrote:\n\n> \n> Signed-off-by: Mike Hommey <mh@glandium.org>\n> ---\n>  http-push.c |   29 ++++++++++-------------------\n>  1 files changed, 10 insertions(+), 19 deletions(-)\n> \n> diff --git a/http-push.c b/http-push.c\n> index e0b4f5a..7627860 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -1960,29 +1960,20 @@ static void update_remote_info_refs(struct remote_lock *lock)\n>  static int remote_exists(const char *path)\n>  {\n\nHeh, I see where your commit subject comes from, but it should rather \nmention the function \"remote_exists()\"...\n\n>  \tchar *url = xmalloc(strlen(remote->url) + strlen(path) + 1);\n> -\tstruct active_request_slot *slot;\n> -\tstruct slot_results results;\n> -\tint ret = -1;\n> +\tint ret;\n>  \n>  \tsprintf(url, \"%s%s\", remote->url, path);\n>  \n> -\tslot = get_active_slot();\n> -\tslot->results = &results;\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n> -\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);\n> -\n> -\tif (start_active_slot(slot)) {\n> -\t\trun_active_slot(slot);\n> -\t\tif (results.http_code == 404)\n> -\t\t\tret = 0;\n> -\t\telse if (results.curl_result == CURLE_OK)\n> -\t\t\tret = 1;\n> -\t\telse\n> -\t\t\tfprintf(stderr, \"HEAD HTTP error %ld\\n\", results.http_code);\n> -\t} else {\n> -\t\tfprintf(stderr, \"Unable to start HEAD request\\n\");\n> +\tswitch (http_get_strbuf(url, NULL, 0)) {\n> +\tcase HTTP_OK:\n> +\t\tret = 1;\n> +\t\tbreak;\n> +\tcase HTTP_MISSING_TARGET:\n> +\t\tret = 0;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tret = -1;\n>  \t}\n\nDoes http_get_strbuf() already show the error?  Not as far as I can see, \neven if it would make sense, no?  At least you'll have to \"return \nerror(...)\".\n\nCiao,\nDscho\n"},{"id":"101036","messageId":"alpine.DEB.1.00.0901182005230.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"1232265877-3649-3-git-send-email-mh@glandium.org","subject":"Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-18T19:06:17Z","receivedAt":"2009-01-18T19:06:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 18 Jan 2009, Mike Hommey wrote:\n\n> diff --git a/transport.c b/transport.c\n> index 56831c5..6919ff1 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n>  \t\tfree(ref);\n>  \t}\n>  \n> +\thttp_cleanup();\n> +\tfree(refs_url);\n>  \treturn refs;\n>  }\n\nYou cannot http_cleanup() here, as http-push calls that function, but \ncontinues to want to use curl.\n\nCiao,\nDscho\n"},{"id":"101037","messageId":"alpine.DEB.1.00.0901182010380.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"alpine.DEB.1.00.0901182005230.3586@pacific.mpi-cbg.de","subject":"Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-18T19:11:07Z","receivedAt":"2009-01-18T19:11:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 18 Jan 2009, Johannes Schindelin wrote:\n\n> On Sun, 18 Jan 2009, Mike Hommey wrote:\n> \n> > diff --git a/transport.c b/transport.c\n> > index 56831c5..6919ff1 100644\n> > --- a/transport.c\n> > +++ b/transport.c\n> > @@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n> >  \t\tfree(ref);\n> >  \t}\n> >  \n> > +\thttp_cleanup();\n> > +\tfree(refs_url);\n> >  \treturn refs;\n> >  }\n> \n> You cannot http_cleanup() here, as http-push calls that function, but \n> continues to want to use curl.\n\nWorse, a http clone will hit the same issue.\n\nCiao,\nDscho\n"},{"id":"101039","messageId":"20090118191928.GA23108@glandium.org","threadId":"17226","inReplyTo":"alpine.DEB.1.00.0901182005230.3586@pacific.mpi-cbg.de","subject":"Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T19:19:28Z","receivedAt":"2009-01-18T19:19:28Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Jan 18, 2009 at 08:06:17PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 18 Jan 2009, Mike Hommey wrote:\n> \n> > diff --git a/transport.c b/transport.c\n> > index 56831c5..6919ff1 100644\n> > --- a/transport.c\n> > +++ b/transport.c\n> > @@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n> >  \t\tfree(ref);\n> >  \t}\n> >  \n> > +\thttp_cleanup();\n> > +\tfree(refs_url);\n> >  \treturn refs;\n> >  }\n> \n> You cannot http_cleanup() here, as http-push calls that function, but \n> continues to want to use curl.\n\nAre you really sure? It doesn't seem so, to me.\n\nMike\n"},{"id":"101040","messageId":"20090118192134.GB23108@glandium.org","threadId":"17226","inReplyTo":"alpine.DEB.1.00.0901181607210.3586@pacific.mpi-cbg.de","subject":"Re: [WIP Patch 04/12] Use the new http API in http_fetch_ref()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T19:21:34Z","receivedAt":"2009-01-18T19:21:34Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Jan 18, 2009 at 04:10:38PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 18 Jan 2009, Mike Hommey wrote:\n> \n> > diff --git a/http.c b/http.c\n> > index 82534cf..0c9504b 100644\n> > --- a/http.c\n> > +++ b/http.c\n> > @@ -604,34 +604,17 @@ int http_fetch_ref(const char *base, struct ref *ref)\n> >  {\n> >  \tchar *url;\n> >  \tstruct strbuf buffer = STRBUF_INIT;\n> > -\tstruct active_request_slot *slot;\n> > -\tstruct slot_results results;\n> > -\tint ret;\n> > +\tint ret = -1;\n> >  \n> >  \turl = quote_ref_url(base, ref->name);\n> > -\tslot = get_active_slot();\n> > -\tslot->results = &results;\n> > -\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);\n> > -\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n> > -\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);\n> > -\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n> > -\tif (start_active_slot(slot)) {\n> > -\t\trun_active_slot(slot);\n> > -\t\tif (results.curl_result == CURLE_OK) {\n> > -\t\t\tstrbuf_rtrim(&buffer);\n> > -\t\t\tif (buffer.len == 40)\n> > -\t\t\t\tret = get_sha1_hex(buffer.buf, ref->old_sha1);\n> > -\t\t\telse if (!prefixcmp(buffer.buf, \"ref: \")) {\n> > -\t\t\t\tref->symref = xstrdup(buffer.buf + 5);\n> > -\t\t\t\tret = 0;\n> > -\t\t\t} else\n> > -\t\t\t\tret = 1;\n> > -\t\t} else {\n> > -\t\t\tret = error(\"Couldn't get %s for %s\\n%s\",\n> > -\t\t\t\t    url, ref->name, curl_errorstr);\n> > +\tif (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) {\n> > +\t\tstrbuf_rtrim(&buffer);\n> > +\t\tif (buffer.len == 40)\n> > +\t\t\tret = get_sha1_hex(buffer.buf, ref->old_sha1);\n> > +\t\telse if (!prefixcmp(buffer.buf, \"ref: \")) {\n> > +\t\t\tref->symref = xstrdup(buffer.buf + 5);\n> > +\t\t\tret = 0;\n> >  \t\t}\n> > -\t} else {\n> > -\t\tret = error(\"Unable to start request\");\n> >  \t}\n> \n> Why not keep that error?\n\nIt should be handled in http_request, I'd say...\n\nMike\n"},{"id":"101041","messageId":"20090118192343.GC23108@glandium.org","threadId":"17226","inReplyTo":"alpine.DEB.1.00.0901181615070.3586@pacific.mpi-cbg.de","subject":"Re: [WIP Patch 08/12] Use the new http API in update_remote_info_refs()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T19:23:43Z","receivedAt":"2009-01-18T19:23:43Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Jan 18, 2009 at 04:18:16PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 18 Jan 2009, Mike Hommey wrote:\n> \n> > \n> > Signed-off-by: Mike Hommey <mh@glandium.org>\n> > ---\n> >  http-push.c |   29 ++++++++++-------------------\n> >  1 files changed, 10 insertions(+), 19 deletions(-)\n> > \n> > diff --git a/http-push.c b/http-push.c\n> > index e0b4f5a..7627860 100644\n> > --- a/http-push.c\n> > +++ b/http-push.c\n> > @@ -1960,29 +1960,20 @@ static void update_remote_info_refs(struct remote_lock *lock)\n> >  static int remote_exists(const char *path)\n> >  {\n> \n> Heh, I see where your commit subject comes from, but it should rather \n> mention the function \"remote_exists()\"...\n> \n> >  \tchar *url = xmalloc(strlen(remote->url) + strlen(path) + 1);\n> > -\tstruct active_request_slot *slot;\n> > -\tstruct slot_results results;\n> > -\tint ret = -1;\n> > +\tint ret;\n> >  \n> >  \tsprintf(url, \"%s%s\", remote->url, path);\n> >  \n> > -\tslot = get_active_slot();\n> > -\tslot->results = &results;\n> > -\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n> > -\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);\n> > -\n> > -\tif (start_active_slot(slot)) {\n> > -\t\trun_active_slot(slot);\n> > -\t\tif (results.http_code == 404)\n> > -\t\t\tret = 0;\n> > -\t\telse if (results.curl_result == CURLE_OK)\n> > -\t\t\tret = 1;\n> > -\t\telse\n> > -\t\t\tfprintf(stderr, \"HEAD HTTP error %ld\\n\", results.http_code);\n> > -\t} else {\n> > -\t\tfprintf(stderr, \"Unable to start HEAD request\\n\");\n> > +\tswitch (http_get_strbuf(url, NULL, 0)) {\n> > +\tcase HTTP_OK:\n> > +\t\tret = 1;\n> > +\t\tbreak;\n> > +\tcase HTTP_MISSING_TARGET:\n> > +\t\tret = 0;\n> > +\t\tbreak;\n> > +\tdefault:\n> > +\t\tret = -1;\n> >  \t}\n> \n> Does http_get_strbuf() already show the error?  Not as far as I can see, \n> even if it would make sense, no?  At least you'll have to \"return \n> error(...)\".\n\nAs I said, it has some error handling regressions ;)\n\nMike\n"},{"id":"101045","messageId":"20090118193002.GA25134@glandium.org","threadId":"17226","inReplyTo":"alpine.DEB.1.00.0901182010380.3586@pacific.mpi-cbg.de","subject":"Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-18T19:30:02Z","receivedAt":"2009-01-18T19:30:02Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Jan 18, 2009 at 08:11:07PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 18 Jan 2009, Johannes Schindelin wrote:\n> \n> > On Sun, 18 Jan 2009, Mike Hommey wrote:\n> > \n> > > diff --git a/transport.c b/transport.c\n> > > index 56831c5..6919ff1 100644\n> > > --- a/transport.c\n> > > +++ b/transport.c\n> > > @@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n> > >  \t\tfree(ref);\n> > >  \t}\n> > >  \n> > > +\thttp_cleanup();\n> > > +\tfree(refs_url);\n> > >  \treturn refs;\n> > >  }\n> > \n> > You cannot http_cleanup() here, as http-push calls that function, but \n> > continues to want to use curl.\n> \n> Worse, a http clone will hit the same issue.\n\nIIRC, it doesn't break anything because getting refs is going to happen\nat the beginning, and the following http request is going to\nreinitialize the whole thing.\n\nIt is suboptimal, I agree.\n\nMike\n"},{"id":"101058","messageId":"alpine.DEB.1.00.0901182202500.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"20090118193002.GA25134@glandium.org","subject":"Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-18T21:09:28Z","receivedAt":"2009-01-18T21:09:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 18 Jan 2009, Mike Hommey wrote:\n\n> On Sun, Jan 18, 2009 at 08:11:07PM +0100, Johannes Schindelin wrote:\n> > Hi,\n> > \n> > On Sun, 18 Jan 2009, Johannes Schindelin wrote:\n> > \n> > > On Sun, 18 Jan 2009, Mike Hommey wrote:\n> > > \n> > > > diff --git a/transport.c b/transport.c\n> > > > index 56831c5..6919ff1 100644\n> > > > --- a/transport.c\n> > > > +++ b/transport.c\n> > > > @@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n> > > >  \t\tfree(ref);\n> > > >  \t}\n> > > >  \n> > > > +\thttp_cleanup();\n> > > > +\tfree(refs_url);\n> > > >  \treturn refs;\n> > > >  }\n> > > \n> > > You cannot http_cleanup() here, as http-push calls that function, but \n> > > continues to want to use curl.\n> > \n> > Worse, a http clone will hit the same issue.\n> \n> IIRC, it doesn't break anything because getting refs is going to happen\n> at the beginning, and the following http request is going to\n> reinitialize the whole thing.\n> \n> It is suboptimal, I agree.\n\n>From the output of t5540-http-push.sh (which tests http clone \nas case 2, incidentally):\n\n-- snip --\n*   ok 1: setup remote repository\n\n* expecting success:\n        cd \"$ROOT_PATH\" &&\n        git clone $HTTPD_URL/test_repo.git test_repo_clone\n\n*** glibc detected *** git: double free or corruption (!prev): \n0x000000000077e090 ***\n======= Backtrace: =========\n/lib/libc.so.6[0x2ba4f5ac8b0a]\n/lib/libc.so.6(cfree+0x8c)[0x2ba4f5acc6fc]\ngit[0x4d7ef2]\ngit[0x4d9073]\ngit[0x4d90b7]\ngit[0x4d1626]\ngit[0x4d1b2f]\ngit[0x4c9abb]\ngit[0x4ca00d]\ngit[0x4cad1a]\ngit[0x41c5f3]\ngit[0x404ee8]\ngit[0x405095]\ngit[0x40517b]\ngit[0x4053b8]\n/lib/libc.so.6(__libc_start_main+0xf4)[0x2ba4f5a74b44]\ngit[0x404589]\n======= Memory map: ========\n00400000-00528000 r-xp 00000000 08:01 45859192                  \n         /home/schindelin/git/git\n00727000-0072d000 rw-p 00127000 08:01 45859192                  \n         /home/schindelin/git/git\n0072d000-007b9000 rw-p 0072d000 00:00 0                         \n         [heap]\n2ba4f4e47000-2ba4f4e64000 r-xp 00000000 08:01 54051869          \n         /lib/ld-2.6.1.so\n2ba4f4e64000-2ba4f4e68000 rw-p 2ba4f4e64000 00:00 0\n2ba4f5063000-2ba4f5065000 rw-p 0001c000 08:01 54051869          \n         /lib/ld-2.6.1.so\n2ba4f5065000-2ba4f50a2000 r-xp 00000000 08:01 49956909          \n         /usr/lib/libcurl.so.4.0.0\n2ba4f50a2000-2ba4f52a1000 ---p 0003d000 08:01 49956909          \n         /usr/lib/libcurl.so.4.0.0\n2ba4f52a1000-2ba4f52a3000 rw-p 0003c000 08:01 49956909          \n         /usr/lib/libcurl.so.4.0.0\n2ba4f52a3000-2ba4f52b9000 r-xp 00000000 08:01 49956837          \n         /usr/lib/libz.so.1.2.3.3\n2ba4f52b9000-2ba4f54b9000 ---p 00016000 08:01 49956837          \n         /usr/lib/libz.so.1.2.3.3\n2ba4f54b9000-2ba4f54ba000 rw-p 00016000 08:01 49956837          \n         /usr/lib/libz.so.1.2.3.3\n2ba4f54ba000-2ba4f5615000 r-xp 00000000 08:01 49957372          \n         /usr/lib/libcrypto.so.0.9.8\n2ba4f5615000-2ba4f5815000 ---p 0015b000 08:01 49957372          \n         /usr/lib/libcrypto.so.0.9.8\n2ba4f5815000-2ba4f5838000 rw-p 0015b000 08:01 49957372          \n         /usr/lib/libcrypto.so.0.9.8\n2ba4f5838000-2ba4f583b000 rw-p 2ba4f5838000 00:00 0\n2ba4f583b000-2ba4f5851000 r-xp 00000000 08:01 54051886          \n         /lib/libpthread-2.6.1.so\n2ba4f5851000-2ba4f5a50000 ---p 00016000 08:01 54051886          \n         /lib/libpthread-2.6.1.so\n2ba4f5a50000-2ba4f5a52000 rw-p 00015000 08:01 54051886          \n         /lib/libpthread-2.6.1.so\n2ba4f5a52000-2ba4f5a57000 rw-p 2ba4f5a52000 00:00 0\n2ba4f5a57000-2ba4f5ba9000 r-xp 00000000 08:01 54051872          \n         /lib/libc-2.6.1.so\n2ba4f5ba9000-2ba4f5da8000 ---p 00152000 08:01 54051872          \n         /lib/libc-2.6.1.so\n2ba4f5da8000-2ba4f5dab000 r--p 00151000 08:01 54051872          \n         /lib/libc-2.6.1.so\n2ba4f5dab000-2ba4f5dad000 rw-p 00154000 08:01 54051872          \n         /lib/libc-2.6.1.so\n2ba4f5dad000-2ba4f5db2000 rw-p 2ba4f5dad000 00:00 0\n2ba4f5db2000-2ba4f5ddc000 r-xp 00000000 08:01 49957359          \n         /usr/lib/libgssapi_krb5.so.2.2\n2ba4f5ddc000-2ba4f5fdb000 ---p 0002a000 08:01 49957359          \n         /usr/lib/libgssapi_krb5.so.2.2\n2ba4f5fdb000-2ba4f5fdd000 rw-p 00029000 08:01 49957359          \n         /usr/lib/libgssapi_krb5.so.2.2\n2ba4f5fdd000-2ba4f600e000 r-xp 00000000 08:01 49957876          \n         /usr/lib/libidn.so.11.5.29\n2ba4f600e000-2ba4f620e000 ---p 00031000 08:01 49957876          \n         /usr/lib/libidn.so.11.5.29\n2ba4f620e000-2ba4f620f000 rw-p 00031000 08:01 49957876          \n         /usr/lib/libidn.so.11.5.29\n2ba4f620f000-2ba4f6210000 rw-p 2ba4f620f000 00:00 0\n2ba4f6210000-2ba4f6212000 r-xp 00000000 08:01 54051875          \n         /lib/libdl-2.6.1.so\n2ba4f6212000-2ba4f6412000 ---p 00002000 08:01 54051875          \n         /lib/libdl-2.6.1.so\n2ba4f6412000-2ba4f6414000 rw-p 00002000 08:01 54051875          \n         /lib/libdl-2.6.1.so\n2ba4f6414000-2ba4f6458000 r-xp 00000000 08:01 49957373          \n         /usr/lib/libssl.so.0.9.8\n2ba4f6458000-2ba4f6657000 ---p 00044000 08:01 49957373          \n         /usr/lib/libssl.so.0.9.8\n2ba4f6657000-2ba4f665d000 rw-p 00043000 08:01 49957373          \n         /usr/lib/libssl.so.0.9.8\n2ba4f665d000-2ba4f66ed000 r-xp 00000000 08:01 49957362          \n         /usr/lib/libkrb5.so.3.3\n2ba4f66ed000-2ba4f68ed000 ---p 00090000 08:01 49957362          \n         /usr/lib/libkrb5.so.3.3\n2ba4f68ed000-2ba4f68f1000 rw-p 00090000 08:01 49957362          \n         /usr/lib/libkrb5.so.3.3\n2ba4f68f1000-2ba4f68f2000 rw-p 2ba4f68f1000 00:00 0\n2ba4f68f2000-2ba4f6916000 r-xp 00000000 08:01 49957360          \n         /usr/lib/libk5crypto.so.3.1\n2ba4f6916000-2ba4f6b15000 ---p 00024000 08:01 49957360          \n         /usr/lib/libk5crypto.so.3.1\n2ba4f6b15000-2ba4f6b17000 rw-p 00023000 08:01 49957360          \n         /usr/lib/libk5crypto.so.3.1\n2ba4f6b17000-2ba4f6b19000 r-xp 00000000 08:01 54050832          \n         /lib/libcom_err.so.2.1\n2ba4f6b19000-2ba4f6d18000 ---p 00002000 08:01 54050832          \n         /lib/libcom_err.so.2.1\n2ba4f6d18000-2ba4f6d19000 rw-p 00001000 08:01 54050832          \n         /lib/libcom_err.so.2.1\n2ba4f6d19000-2ba4f6d20000 r-xp 00000000 08:01 49957363          \n         /usr/lib/libkrb5support.so.0.1\n2ba4f6d20000-2ba4f6f20000 ---p 00007000 08:01 49957363          \n         /usr/lib/libkrb5support.so.0.1\n2ba4f6f20000-2ba4f6f21000 rw-p 00007000 08:01 49957363          \n         /usr/lib/libkrb5support.so.0.1\n2ba4f6f21000-2ba4f6f22000 rw-p 2ba4f6f21000 00:00 0\n2ba4f6f22000-2ba4f6f24000 r-xp 00000000 08:01 54050958          \n         /lib/libkeyutils-1.2.so\n2ba4f6f24000-2ba4f7123000 ---p 00002000 08:01 54050958          \n         /lib/libkeyutils-1.2.so\n2ba4f7123000-2ba4f7124000 rw-p 00001000 08:01 54050958          \n         /lib/libkeyutils-1.2.so\n2ba4f7124000-2ba4f7136error: Request for \n0c973ae9bd51902a28466f3850b543fa66a6aaf4 aborted\nInitialized empty Git repository in \n/home/schindelin/git/t/trash \ndirectory.t5540-http-push/test_repo_clone/.git/\nAborted\n* FAIL 2: clone remote repository\n\n                cd \"$ROOT_PATH\" &&\n                git clone $HTTPD_URL/test_repo.git test_repo_clone\n-- snap --\n\nIt might be a strange interaction with my patches, though.  valgrind \npointed to the http_cleanup() call, and after removing that call, the \nmessage goes away.\n\nCiao,\nDscho\n"},{"id":"101059","messageId":"alpine.DEB.1.00.0901182210190.3586@pacific.mpi-cbg.de","threadId":"17226","inReplyTo":"20090118191928.GA23108@glandium.org","subject":"Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-18T21:10:44Z","receivedAt":"2009-01-18T21:10:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 18 Jan 2009, Mike Hommey wrote:\n\n> On Sun, Jan 18, 2009 at 08:06:17PM +0100, Johannes Schindelin wrote:\n> \n> > You cannot http_cleanup() here, as http-push calls that function, but \n> > continues to want to use curl.\n> \n> Are you really sure? It doesn't seem so, to me.\n\nNope, sorry.  It was http clone.\n\nCiao,\nDscho\n"}]}