{"thread":{"id":"19051","subject":"[PATCH 3/3] http-push: send out fetch requests on queue","startedAt":"2009-04-24T16:35:57Z","lastAt":"2009-06-02T13:55:23Z","messageCount":15,"participants":["Tay Ray Chuan","Clemens Buchacher","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"112215","messageId":"49F1EA6D.8080406@gmail.com","threadId":"19051","inReplyTo":null,"subject":"[PATCH 3/3] http-push: send out fetch requests on queue","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-04-24T16:35:57Z","receivedAt":"2009-04-24T16:35:57Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Previously, requests for remote files were simply added to the queue\n(pointed to by request_queue_head) and no transfer actually took\nplace[1], even though code that followed may rely on these remote files\nto be present (eg. the setup_revisions invocation).\n\nThe code that sends out the requests on the request queue is refactored\ninto the method run_request_queue.\n\nAfter the get_dav_remote_heads invocation (ie. after fetch requests are\nadded to the queue), the requests on the queue are sent out through an\ninvocation to run_request_queue.\n\nThis invocation to run_request_queue entails adding a fill function\nbefore pushing checks take place, which may lead to accidental,\nunwanted pushes previously.\n\nThe flag is_running_queue is introduced to prevent this from occurring.\nfill_active_slot is made to check the flag is_running_queue before\nthe sending of the requests proceeds.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n http-push.c          |   37 ++++++++++++++++++++++++++-----------\n t/t5540-http-push.sh |    4 ++--\n 2 files changed, 28 insertions(+), 13 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 5138224..1ba7987 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -843,11 +843,12 @@ static void finish_request(struct transfer_request *request)\n }\n\n #ifdef USE_CURL_MULTI\n+static int is_running_queue;\n static int fill_active_slot(void *unused)\n {\n \tstruct transfer_request *request;\n\n-\tif (aborted)\n+\tif (aborted || !is_running_queue)\n \t\treturn 0;\n\n \tfor (request = request_queue_head; request; request = request->next) {\n@@ -2167,6 +2168,25 @@ static int delete_remote_branch(char *pattern, int force)\n \treturn 0;\n }\n\n+void run_request_queue()\n+{\n+#ifdef USE_CURL_MULTI\n+\tis_running_queue = 1;\n+\tfill_active_slots();\n+\tadd_fill_function(NULL, fill_active_slot);\n+#endif\n+\tdo {\n+\t\tfinish_all_active_slots();\n+#ifdef USE_CURL_MULTI\n+\t\tfill_active_slots();\n+#endif\n+\t} while (request_queue_head && !aborted);\n+\n+#ifdef USE_CURL_MULTI\n+\tis_running_queue = 0;\n+#endif\n+}\n+\n int main(int argc, char **argv)\n {\n \tstruct transfer_request *request;\n@@ -2271,6 +2291,8 @@ int main(int argc, char **argv)\n \t\trepo->url = rewritten_url;\n \t}\n\n+\tis_running_queue = 0;\n+\n \t/* Verify DAV compliance/lock support */\n \tif (!locking_available()) {\n \t\trc = 1;\n@@ -2300,6 +2322,7 @@ int main(int argc, char **argv)\n \tlocal_refs = get_local_heads();\n \tfprintf(stderr, \"Fetching remote heads...\\n\");\n \tget_dav_remote_heads();\n+\trun_request_queue();\n\n \t/* Remove a remote branch if -d or -D was specified */\n \tif (delete_branch) {\n@@ -2429,16 +2452,8 @@ int main(int argc, char **argv)\n \t\tif (objects_to_send)\n \t\t\tfprintf(stderr, \"    sending %d objects\\n\",\n \t\t\t\tobjects_to_send);\n-#ifdef USE_CURL_MULTI\n-\t\tfill_active_slots();\n-\t\tadd_fill_function(NULL, fill_active_slot);\n-#endif\n-\t\tdo {\n-\t\t\tfinish_all_active_slots();\n-#ifdef USE_CURL_MULTI\n-\t\t\tfill_active_slots();\n-#endif\n-\t\t} while (request_queue_head && !aborted);\n+\n+\t\trun_request_queue();\n\n \t\t/* Update the remote branch if all went well */\n \t\tif (aborted || !update_remote(ref->new_sha1, ref_lock))\ndiff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh\nindex ad0f14b..f4a2cf6 100755\n--- a/t/t5540-http-push.sh\n+++ b/t/t5540-http-push.sh\n@@ -67,7 +67,7 @@ test_expect_success ' push to remote repository with unpacked refs' '\n \t test $HEAD = $(git rev-parse --verify HEAD))\n '\n\n-test_expect_failure 'http-push fetches unpacked objects' '\n+test_expect_success 'http-push fetches unpacked objects' '\n \tcp -R \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git \\\n \t\t\"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_unpacked.git &&\n\n@@ -83,7 +83,7 @@ test_expect_failure 'http-push fetches unpacked objects' '\n \t git push -f -v $HTTPD_URL/test_repo_unpacked.git master)\n '\n\n-test_expect_failure 'http-push fetches packed objects' '\n+test_expect_success 'http-push fetches packed objects' '\n \tcp -R \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git \\\n \t\t\"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_packed.git &&\n\n-- \n1.6.3.rc0\n"},{"id":"115073","messageId":"20090530091755.GA13578@localhost","threadId":"19051","inReplyTo":"49F1EA6D.8080406@gmail.com","subject":"Re: [PATCH 3/3] http-push: send out fetch requests on queue","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-05-30T09:17:55Z","receivedAt":"2009-05-30T09:17:55Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Hi,\n\nOn Sat, Apr 25, 2009 at 12:35:57AM +0800, Tay Ray Chuan wrote:\n[...]\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -2300,6 +2322,7 @@ int main(int argc, char **argv)\n>  \tlocal_refs = get_local_heads();\n>  \tfprintf(stderr, \"Fetching remote heads...\\n\");\n>  \tget_dav_remote_heads();\n> +\trun_request_queue();\n> \n>  \t/* Remove a remote branch if -d or -D was specified */\n>  \tif (delete_branch) {\n\nThe \"http-push fetches packed objects\" test of t5540 on pu either segfaults\nor hangs indefinitely on my machine [1]. The bug is revealed by the added\nline above in commit 0d71b15 (http-push: send out fetch requests on queue).\n\nI have been trying to debug this, but I'm at a loss. Below are the backtrace\nand an excerpt of the valgrind output with 0d71b15. See\nhttp://gist.github.com/120453 for the full output. Note that the\nsegfault/hang does not happen if valgrind is used.\n\nCan anybody reproduce this? Set GIT_TEST_HTTPD to enable this test.\n\nClemens\n\n[1] Debian amd64 stable (mostly), libcurl version 17.18.2\n---\n\n(gdb) r\nStarting program: /home/drizzd/fridge/linux/src/git/git-http-push --force\n--verbose http://127.0.0.1:5540/test_repo_packed.git master\n[Thread debugging using libthread_db enabled]\nGetting pack list\nGetting index for pack ecba592576b66d9aa805891d0a80a0133abd5f5d\nFetching remote heads...\n  refs/\n  refs/heads/\n  fetch 9d498b0bbc2a25438e2fbd19081948da86028c23 for refs/heads/master\n  refs/tags/\nFetching pack ecba592576b66d9aa805891d0a80a0133abd5f5d\n which contains 9d498b0bbc2a25438e2fbd19081948da86028c23\n[New Thread 0x7f60404a9700 (LWP 21395)]\n\nProgram received signal SIGSEGV, Segmentation fault.\n[Switching to Thread 0x7f60404a9700 (LWP 21395)]\n0x00007f603f49245e in ?? () from /lib/libc.so.6\n(gdb) bt\n#0  0x00007f603f49245e in ?? () from /lib/libc.so.6\n#1  0x00007f603f49104a in ftell () from /lib/libc.so.6\n#2  0x0000000000409785 in run_active_slot (slot=0x6bab40) at http.c:536\n#3  0x00000000004099f0 in finish_all_active_slots () at http.c:607\n#4  0x000000000040f72f in run_request_queue () at http-push.c:2179\n#5  0x000000000040fcb4 in main (argc=5, argv=0x7fff485cd748) at\nhttp-push.c:2325\n\n\n* checking known breakage: \n\tcp -R \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git \\\n\t\t\"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_packed.git &&\n\n\tgit clone $HTTPD_URL/test_repo_packed.git \\\n\t\t\"$ROOT_PATH\"/test_repo_clone_packed &&\n\n\t(cd \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo_packed.git &&\n\t git --bare repack &&\n\t git --bare prune-packed) &&\n\n\t# By reset, we force git to retrieve the packed object\n\t(cd \"$ROOT_PATH\"/test_repo_clone_packed &&\n\t git reset --hard HEAD^ &&\n\t git remote rm origin &&\n\t git reflog expire --expire=0 --all &&\n\t git prune &&\n\t valgrind git-http-push --force --verbose $HTTPD_URL/test_repo_packed.git master)\n\ngot 9d498b0bbc2a25438e2fbd19081948da86028c23\nwalk 9d498b0bbc2a25438e2fbd19081948da86028c23\ngot 50b820aea6d3503362343cdc0e699b760c700b2b\ngot 0c973ae9bd51902a28466f3850b543fa66a6aaf4\ngot e69de29bb2d1d6434b8b29ae775ad8c2e48c5391\nwalk 0c973ae9bd51902a28466f3850b543fa66a6aaf4\ngot 146e9fd4040abc05619b6c07a3a23cba27a2c722\nInitialized empty Git repository in /home/drizzd/fridge/linux/src/git/t/trash directory.t5540-http-push/test_repo_clone_packed/.git/\nHEAD is now at 0c973ae initial\n==== Memcheck, a memory error detector.\n==== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.\n==== Using LibVEX rev 1854, a library for dynamic binary translation.\n==== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.\n==== Using valgrind-3.3.1-Debian, a dynamic binary instrumentation framework.\n==== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.\n==== For more details, rerun with: -v\n==== \n==== Invalid read of size 1\n====    at 0x409EBE: xml_entities (http-push.c:192)\n====    by 0x40DE29: locking_available (http-push.c:1619)\n====    by 0x40FBCF: main (http-push.c:2297)\n====  Address 0x80b94e4 is 0 bytes after a block of size 44 alloc'd\n====    at 0x4C2260E: malloc (vg_replace_malloc.c:207)\n====    by 0x44C4E4: xmalloc (wrapper.c:20)\n====    by 0x40FB4D: main (http-push.c:2286)\nGetting pack list\nGetting index for pack ecba592576b66d9aa805891d0a80a0133abd5f5d\nFetching remote heads...\n  refs/\n  refs/heads/\n  fetch 9d498b0bbc2a25438e2fbd19081948da86028c23 for refs/heads/master\n  refs/tags/\nFetching pack ecba592576b66d9aa805891d0a80a0133abd5f5d\n which contains 9d498b0bbc2a25438e2fbd19081948da86028c23\n==== \n==== Invalid read of size 2\n====    at 0x5ABFFE1: ftell (in /lib/libc-2.7.so)\n====    by 0x409784: run_active_slot (http.c:536)\n====    by 0x4099EF: finish_all_active_slots (http.c:607)\n====    by 0x40F72E: run_request_queue (http-push.c:2179)\n====    by 0x40FCB3: main (http-push.c:2325)\n====  Address 0x817c688 is 0 bytes inside a block of size 568 free'd\n====    at 0x4C2130F: free (vg_replace_malloc.c:323)\n====    by 0x5ABEDA0: fclose (in /lib/libc-2.7.so)\n====    by 0x40BC64: finish_request (http-push.c:824)\n====    by 0x40A0F4: process_response (http-push.c:257)\n====    by 0x4099C4: finish_active_slot (http.c:598)\n====    by 0x4088E3: process_curl_messages (http.c:101)\n====    by 0x409709: step_active_slots (http.c:512)\n====    by 0x40975A: run_active_slot (http.c:533)\n====    by 0x4099EF: finish_all_active_slots (http.c:607)\n====    by 0x40F72E: run_request_queue (http-push.c:2179)\n====    by 0x40FCB3: main (http-push.c:2325)\n[...]\n==== Invalid read of size 4\n====    at 0x5AC00AC: ftell (in /lib/libc-2.7.so)\n====    by 0x409784: run_active_slot (http.c:536)\n====    by 0x4099EF: finish_all_active_slots (http.c:607)\n====    by 0x40F72E: run_request_queue (http-push.c:2179)\n====    by 0x40FCB3: main (http-push.c:2325)\n====  Address 0x817c768 is 224 bytes inside a block of size 568 free'd\n====    at 0x4C2130F: free (vg_replace_malloc.c:323)\n====    by 0x5ABEDA0: fclose (in /lib/libc-2.7.so)\n====    by 0x40BC64: finish_request (http-push.c:824)\n====    by 0x40A0F4: process_response (http-push.c:257)\n====    by 0x4099C4: finish_active_slot (http.c:598)\n====    by 0x4088E3: process_curl_messages (http.c:101)\n====    by 0x409709: step_active_slots (http.c:512)\n====    by 0x40975A: run_active_slot (http.c:533)\n====    by 0x4099EF: finish_all_active_slots (http.c:607)\n====    by 0x40F72E: run_request_queue (http-push.c:2179)\n====    by 0x40FCB3: main (http-push.c:2325)\nupdating 'refs/heads/master'\n  from 9d498b0bbc2a25438e2fbd19081948da86028c23\n  to   0c973ae9bd51902a28466f3850b543fa66a6aaf4\n    done\nUpdating remote server info\n==== \n==== ERROR SUMMARY: 46 errors from 42 contexts (suppressed: 8 from 1)\n==== malloc/free: in use at exit: 201,408 bytes in 54 blocks.\n==== malloc/free: 2,976 allocs, 2,922 frees, 1,512,372 bytes allocated.\n==== For counts of detected errors, rerun with: -v\n==== searching for pointers to 54 not-freed blocks.\n==== checked 648,992 bytes.\n==== \n==== LEAK SUMMARY:\n====    definitely lost: 10,463 bytes in 26 blocks.\n====      possibly lost: 0 bytes in 0 blocks.\n====    still reachable: 190,945 bytes in 28 blocks.\n====         suppressed: 0 bytes in 0 blocks.\n==== Rerun with --leak-check=full to see details of leaked memory.\n*   FIXED 6: http-push fetches packed objects\n\n* fixed 1 known breakage(s)\n* still have 1 known breakage(s)\n* passed all remaining 5 test(s)\n"},{"id":"115074","messageId":"be6fef0d0905300231k5167f3efle9a450419bdfa1cb@mail.gmail.com","threadId":"19051","inReplyTo":"20090530091755.GA13578@localhost","subject":"Re: [PATCH 3/3] http-push: send out fetch requests on queue","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-05-30T09:31:36Z","receivedAt":"2009-05-30T09:31:36Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sat, May 30, 2009 at 5:17 PM, Clemens Buchacher <drizzd@aon.at> wrote:\n> The \"http-push fetches packed objects\" test of t5540 on pu either segfaults\n> or hangs indefinitely on my machine [1]. The bug is revealed by the added\n> line above in commit 0d71b15 (http-push: send out fetch requests on queue).\n\nif you don't mind, could you try running this test on the\nhttp-fix-push-fetching branch in git://github.com/rctay/git.git and\nsee if this still happens? It contains these patches only, applied on\nmaster.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"115075","messageId":"20090530093717.GA22129@localhost","threadId":"19051","inReplyTo":"be6fef0d0905300231k5167f3efle9a450419bdfa1cb@mail.gmail.com","subject":"Re: [PATCH 3/3] http-push: send out fetch requests on queue","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-05-30T09:37:17Z","receivedAt":"2009-05-30T09:37:17Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, May 30, 2009 at 05:31:36PM +0800, Tay Ray Chuan wrote:\n> On Sat, May 30, 2009 at 5:17 PM, Clemens Buchacher <drizzd@aon.at> wrote:\n> > The \"http-push fetches packed objects\" test of t5540 on pu either segfaults\n> > or hangs indefinitely on my machine [1]. The bug is revealed by the added\n> > line above in commit 0d71b15 (http-push: send out fetch requests on queue).\n> \n> if you don't mind, could you try running this test on the\n> http-fix-push-fetching branch in git://github.com/rctay/git.git and\n> see if this still happens? It contains these patches only, applied on\n> master.\n\nSure. The same thing happens.\n"},{"id":"115080","messageId":"be6fef0d0905300352o33694420m9c988daa554420a3@mail.gmail.com","threadId":"19051","inReplyTo":"20090530093717.GA22129@localhost","subject":"Re: [PATCH 3/3] http-push: send out fetch requests on queue","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-05-30T10:52:07Z","receivedAt":"2009-05-30T10:52:07Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sat, May 30, 2009 at 5:37 PM, Clemens Buchacher <drizzd@aon.at> wrote:\n> Sure. The same thing happens.\n\ncuriously, I wasn't able to reproduce on my cygwin setup, but on my ubuntu box.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"115087","messageId":"20090530230153.527532b0.rctay89@gmail.com","threadId":"19051","inReplyTo":"be6fef0d0905300352o33694420m9c988daa554420a3@mail.gmail.com","subject":"Re: [PATCH 3/3] http-push: send out fetch requests on queue","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-05-30T15:01:53Z","receivedAt":"2009-05-30T15:01:53Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Set slot->local to NULL after doing a fclose on the FILE* pointer it\npoints to.\n\nMove NULL assignment to request->slot from\nhttp-push.c::finish_request() to http-push.c::release_request(). This\nis safe, since the functions finish_request() invoke will overwrite\nrequest->slot anyway.\n\nRefactor http-push.c::fetch_index(), http-walker.c::fetch_index() and\nhttp-walker.c::fetch_pack() to use labels while cleaning up.\n\nThis fixes the issue raised by Clemens Buchacher on 30th May:\n\n  http://www.spinics.net/lists/git/msg104623.html\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n\nThis applies on master.\n\n http-push.c   |   23 ++++++++++++++---------\n http-walker.c |   36 +++++++++++++++++++++++-------------\n 2 files changed, 37 insertions(+), 22 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex dac2c6e..816824a 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -715,6 +715,8 @@ static void release_request(struct transfer_request *request)\n \t\tclose(request->local_fileno);\n \tif (request->local_stream)\n \t\tfclose(request->local_stream);\n+\tif (request->slot)\n+\t\trequest->slot = NULL;\n \tfree(request->url);\n \tfree(request);\n }\n@@ -727,7 +729,6 @@ static void finish_request(struct transfer_request *request)\n \n \trequest->curl_result = request->slot->curl_result;\n \trequest->http_code = request->slot->http_code;\n-\trequest->slot = NULL;\n \n \t/* Keep locks active */\n \tcheck_locks();\n@@ -823,6 +824,7 @@ static void finish_request(struct transfer_request *request)\n \n \t\t\tfclose(request->local_stream);\n \t\t\trequest->local_stream = NULL;\n+\t\t\trequest->slot->local = 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@@ -946,6 +948,7 @@ static int add_send_request(struct object *obj, struct remote_lock *lock)\n \n static int fetch_index(unsigned char *sha1)\n {\n+\tint ret;\n \tchar *hex = sha1_to_hex(sha1);\n \tchar *filename;\n \tchar *url;\n@@ -1022,21 +1025,23 @@ static int fetch_index(unsigned char *sha1)\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\tret = error(\"Unable to get pack index %s\\n%s\", url,\n+\t\t\t\t    curl_errorstr);\n+\t\t\tgoto cleanup;\n \t\t}\n \t} else {\n-\t\tfree(url);\n-\t\tfclose(indexfile);\n-\t\treturn error(\"Unable to start request\");\n+\t\tret = error(\"Unable to start request\");\n+\t\tgoto cleanup;\n \t}\n \n+\tret = move_temp_to_file(tmpfile, filename);\n+\n+cleanup:\n \tfree(url);\n \tfclose(indexfile);\n+\tslot->local = NULL;\n \n-\treturn move_temp_to_file(tmpfile, filename);\n+\treturn ret;\n }\n \n static int setup_index(unsigned char *sha1)\ndiff --git a/http-walker.c b/http-walker.c\nindex 7321ccc..779947d 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -364,6 +364,7 @@ static void prefetch(struct walker *walker, unsigned char *sha1)\n \n static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1)\n {\n+\tint ret;\n \tchar *hex = sha1_to_hex(sha1);\n \tchar *filename;\n \tchar *url;\n@@ -417,18 +418,21 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch\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\tret = error(\"Unable to get pack index %s\\n%s\", url,\n+\t\t\t\t    curl_errorstr);\n+\t\t\tgoto cleanup;\n \t\t}\n \t} else {\n-\t\tfclose(indexfile);\n-\t\treturn error(\"Unable to start request\");\n+\t\tret = error(\"Unable to start request\");\n+\t\tgoto cleanup;\n \t}\n \n-\tfclose(indexfile);\n+\tret = move_temp_to_file(tmpfile, filename);\n \n-\treturn move_temp_to_file(tmpfile, filename);\n+cleanup:\n+\tfclose(indexfile);\n+\tslot->local = NULL;\n+\treturn ret;\n }\n \n static int setup_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1)\n@@ -718,7 +722,7 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha\n \tFILE *packfile;\n \tchar *filename;\n \tchar tmpfile[PATH_MAX];\n-\tint ret;\n+\tint ret = 0;\n \tlong prev_posn = 0;\n \tchar range[RANGE_HEADER_SIZE];\n \tstruct curl_slist *range_header = NULL;\n@@ -775,17 +779,23 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha\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\tret = error(\"Unable to get pack file %s\\n%s\", url,\n+\t\t\t\t    curl_errorstr);\n+\t\t\tgoto cleanup;\n \t\t}\n \t} else {\n-\t\tfclose(packfile);\n-\t\treturn error(\"Unable to start request\");\n+\t\tret = error(\"Unable to start request\");\n+\t\tgoto cleanup;\n \t}\n \n \ttarget->pack_size = ftell(packfile);\n+\n+cleanup:\n \tfclose(packfile);\n+\tslot->local = NULL;\n+\n+\tif (ret)\n+\t\treturn ret;\n \n \tret = move_temp_to_file(tmpfile, filename);\n \tif (ret)\n-- \n1.6.3.1\n"},{"id":"115089","messageId":"20090531000955.953725d9.rctay89@gmail.com","threadId":"19051","inReplyTo":"20090530230153.527532b0.rctay89@gmail.com","subject":"[PATCH] http*: cleanup slot->local after fclose","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-05-30T16:09:55Z","receivedAt":"2009-05-30T16:09:55Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Set slot->local to NULL after doing a fclose on the FILE* pointer it\npoints to.\n\nMove NULL assignment to request->slot from\nhttp-push.c::finish_request() to http-push.c::release_request(). This\nis safe, since the functions finish_request() invoke will overwrite\nrequest->slot anyway.\n\nRefactor http-push.c::fetch_index(), http-walker.c::fetch_index() and\nhttp-walker.c::fetch_pack() to use labels while cleaning up.\n\nThis fixes the issue raised by Clemens Buchacher on 30th May:\n\n  http://www.spinics.net/lists/git/msg104623.html\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n\n This applies on master, and is a resend, this time with the subject.\n\n http-push.c   |   23 ++++++++++++++---------\n http-walker.c |   36 +++++++++++++++++++++++-------------\n 2 files changed, 37 insertions(+), 22 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex dac2c6e..816824a 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -715,6 +715,8 @@ static void release_request(struct transfer_request *request)\n \t\tclose(request->local_fileno);\n \tif (request->local_stream)\n \t\tfclose(request->local_stream);\n+\tif (request->slot)\n+\t\trequest->slot = NULL;\n \tfree(request->url);\n \tfree(request);\n }\n@@ -727,7 +729,6 @@ static void finish_request(struct transfer_request *request)\n \n \trequest->curl_result = request->slot->curl_result;\n \trequest->http_code = request->slot->http_code;\n-\trequest->slot = NULL;\n \n \t/* Keep locks active */\n \tcheck_locks();\n@@ -823,6 +824,7 @@ static void finish_request(struct transfer_request *request)\n \n \t\t\tfclose(request->local_stream);\n \t\t\trequest->local_stream = NULL;\n+\t\t\trequest->slot->local = 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@@ -946,6 +948,7 @@ static int add_send_request(struct object *obj, struct remote_lock *lock)\n \n static int fetch_index(unsigned char *sha1)\n {\n+\tint ret;\n \tchar *hex = sha1_to_hex(sha1);\n \tchar *filename;\n \tchar *url;\n@@ -1022,21 +1025,23 @@ static int fetch_index(unsigned char *sha1)\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\tret = error(\"Unable to get pack index %s\\n%s\", url,\n+\t\t\t\t    curl_errorstr);\n+\t\t\tgoto cleanup;\n \t\t}\n \t} else {\n-\t\tfree(url);\n-\t\tfclose(indexfile);\n-\t\treturn error(\"Unable to start request\");\n+\t\tret = error(\"Unable to start request\");\n+\t\tgoto cleanup;\n \t}\n \n+\tret = move_temp_to_file(tmpfile, filename);\n+\n+cleanup:\n \tfree(url);\n \tfclose(indexfile);\n+\tslot->local = NULL;\n \n-\treturn move_temp_to_file(tmpfile, filename);\n+\treturn ret;\n }\n \n static int setup_index(unsigned char *sha1)\ndiff --git a/http-walker.c b/http-walker.c\nindex 7321ccc..779947d 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -364,6 +364,7 @@ static void prefetch(struct walker *walker, unsigned char *sha1)\n \n static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1)\n {\n+\tint ret;\n \tchar *hex = sha1_to_hex(sha1);\n \tchar *filename;\n \tchar *url;\n@@ -417,18 +418,21 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch\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\tret = error(\"Unable to get pack index %s\\n%s\", url,\n+\t\t\t\t    curl_errorstr);\n+\t\t\tgoto cleanup;\n \t\t}\n \t} else {\n-\t\tfclose(indexfile);\n-\t\treturn error(\"Unable to start request\");\n+\t\tret = error(\"Unable to start request\");\n+\t\tgoto cleanup;\n \t}\n \n-\tfclose(indexfile);\n+\tret = move_temp_to_file(tmpfile, filename);\n \n-\treturn move_temp_to_file(tmpfile, filename);\n+cleanup:\n+\tfclose(indexfile);\n+\tslot->local = NULL;\n+\treturn ret;\n }\n \n static int setup_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1)\n@@ -718,7 +722,7 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha\n \tFILE *packfile;\n \tchar *filename;\n \tchar tmpfile[PATH_MAX];\n-\tint ret;\n+\tint ret = 0;\n \tlong prev_posn = 0;\n \tchar range[RANGE_HEADER_SIZE];\n \tstruct curl_slist *range_header = NULL;\n@@ -775,17 +779,23 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha\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\tret = error(\"Unable to get pack file %s\\n%s\", url,\n+\t\t\t\t    curl_errorstr);\n+\t\t\tgoto cleanup;\n \t\t}\n \t} else {\n-\t\tfclose(packfile);\n-\t\treturn error(\"Unable to start request\");\n+\t\tret = error(\"Unable to start request\");\n+\t\tgoto cleanup;\n \t}\n \n \ttarget->pack_size = ftell(packfile);\n+\n+cleanup:\n \tfclose(packfile);\n+\tslot->local = NULL;\n+\n+\tif (ret)\n+\t\treturn ret;\n \n \tret = move_temp_to_file(tmpfile, filename);\n \tif (ret)\n-- \n1.6.3.1\n"},{"id":"115093","messageId":"20090530165807.GA10132@localhost","threadId":"19051","inReplyTo":"20090531000955.953725d9.rctay89@gmail.com","subject":"Re: [PATCH] http*: cleanup slot->local after fclose","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-05-30T16:58:07Z","receivedAt":"2009-05-30T16:58:07Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, May 31, 2009 at 12:09:55AM +0800, Tay Ray Chuan wrote:\n> Set slot->local to NULL after doing a fclose on the FILE* pointer it\n> points to.\n\nThanks. That fixes it.\n\nClemens\n"},{"id":"115115","messageId":"7vy6sdssnk.fsf@alter.siamese.dyndns.org","threadId":"19051","inReplyTo":"20090531000955.953725d9.rctay89@gmail.com","subject":"Re: [PATCH] http*: cleanup slot->local after fclose","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-31T06:17:19Z","receivedAt":"2009-05-31T06:17:19Z","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> Set slot->local to NULL after doing a fclose on the FILE* pointer it\n> points to.\n>\n> Move NULL assignment to request->slot from\n> http-push.c::finish_request() to http-push.c::release_request(). This\n> is safe, since the functions finish_request() invoke will overwrite\n> request->slot anyway.\n>\n> Refactor http-push.c::fetch_index(), http-walker.c::fetch_index() and\n> http-walker.c::fetch_pack() to use labels while cleaning up.\n>\n> This fixes the issue raised by Clemens Buchacher on 30th May:\n>\n>   http://www.spinics.net/lists/git/msg104623.html\n>\n> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n> ---\n>\n>  This applies on master, and is a resend, this time with the subject.\n\nShouldn't a fix instead be queued for 'maint', without \"refactoring\"?\n\nIs there a reproducible repipe you can add to the test script?\n\n>  http-push.c   |   23 ++++++++++++++---------\n>  http-walker.c |   36 +++++++++++++++++++++++-------------\n>  2 files changed, 37 insertions(+), 22 deletions(-)\n>\n> diff --git a/http-push.c b/http-push.c\n> index dac2c6e..816824a 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -715,6 +715,8 @@ static void release_request(struct transfer_request *request)\n>  \t\tclose(request->local_fileno);\n>  \tif (request->local_stream)\n>  \t\tfclose(request->local_stream);\n> +\tif (request->slot)\n> +\t\trequest->slot = NULL;\n>  \tfree(request->url);\n>  \tfree(request);\n\nHmm, what's the point of setting NULL to request->slot if you are already\nfreeing \"request\" that contains the field?\n"},{"id":"115125","messageId":"be6fef0d0905310148g7ee7a642jf6a9661d6e62b40b@mail.gmail.com","threadId":"19051","inReplyTo":"7vy6sdssnk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] http*: cleanup slot->local after fclose","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-05-31T08:48:58Z","receivedAt":"2009-05-31T08:48:58Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sun, May 31, 2009 at 2:17 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Shouldn't a fix instead be queued for 'maint', without \"refactoring\"?\n\none on the way.\n\n> Is there a reproducible repipe you can add to the test script?\n\nwhat do you mean by \"repipe\"? To my mind, reproducing Clemens' issue\nwould require some code manipulation and compiling.\n\n> Hmm, what's the point of setting NULL to request->slot if you are already\n> freeing \"request\" that contains the field?\n\nyou do have a point. :)\n\n-- \nCheers,\nRay Chuan\n"},{"id":"115127","messageId":"20090531175413.962a55c3.rctay89@gmail.com","threadId":"19051","inReplyTo":"7vy6sdssnk.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] http*: cleanup slot->local after fclose","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-05-31T09:54:13Z","receivedAt":"2009-05-31T09:54:13Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Set slot->local to NULL after doing a fclose on the FILE* pointer it\npoints to.\n\nThis fixes the issue raised by Clemens Buchacher on 30th May:\n\n  http://www.spinics.net/lists/git/msg104623.html\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n\nOn Sat, 30 May 2009 23:17:19 -0700\nJunio C Hamano <gitster@pobox.com> wrote:\n> Shouldn't a fix instead be queued for 'maint', without \"refactoring\"?\n\nI tested this on top of the first 4 patches in 'rc/http-push' in 'pu',\napplied on 'maint'. [1] I wonder if this should instead be queued for\n'pu' [2], since the issue only occurs there, although, conceivably, it\n*could* happen without those patches in 'pu'.\n\nFootnotes:\n[1] You can find this in the 'http-cleanup-slot-local_maint branch' at\n    git://github.com/rctay/git.git. It's quite a minimal testcase, if\n    you're looking for one.\n\n[2] Found in 'http-cleanup-slot-local_pu'.\n\n http-push.c   |    6 ++++++\n http-walker.c |    6 ++++++\n 2 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex e16a0ad..0b12ffe 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -724,9 +724,11 @@ static void finish_request(struct transfer_request *request)\n \tstruct stat st;\n \tstruct packed_git *target;\n \tstruct packed_git **lst;\n+\tstruct active_request_slot *slot;\n \n \trequest->curl_result = request->slot->curl_result;\n \trequest->http_code = request->slot->http_code;\n+\tslot = request->slot;\n \trequest->slot = NULL;\n \n \t/* Keep locks active */\n@@ -823,6 +825,7 @@ static void finish_request(struct transfer_request *request)\n \n \t\t\tfclose(request->local_stream);\n \t\t\trequest->local_stream = NULL;\n+\t\t\tslot->local = 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@@ -1024,17 +1027,20 @@ static int fetch_index(unsigned char *sha1)\n \t\tif (results.curl_result != CURLE_OK) {\n \t\t\tfree(url);\n \t\t\tfclose(indexfile);\n+\t\t\tslot->local = NULL;\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\tslot->local = NULL;\n \t\treturn error(\"Unable to start request\");\n \t}\n \n \tfree(url);\n \tfclose(indexfile);\n+\tslot->local = NULL;\n \n \treturn move_temp_to_file(tmpfile, filename);\n }\ndiff --git a/http-walker.c b/http-walker.c\nindex 7321ccc..9377851 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -418,15 +418,18 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch\n \t\trun_active_slot(slot);\n \t\tif (results.curl_result != CURLE_OK) {\n \t\t\tfclose(indexfile);\n+\t\t\tslot->local = NULL;\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\tslot->local = NULL;\n \t\treturn error(\"Unable to start request\");\n \t}\n \n \tfclose(indexfile);\n+\tslot->local = NULL;\n \n \treturn move_temp_to_file(tmpfile, filename);\n }\n@@ -776,16 +779,19 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha\n \t\trun_active_slot(slot);\n \t\tif (results.curl_result != CURLE_OK) {\n \t\t\tfclose(packfile);\n+\t\t\tslot->local = NULL;\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\tslot->local = NULL;\n \t\treturn error(\"Unable to start request\");\n \t}\n \n \ttarget->pack_size = ftell(packfile);\n \tfclose(packfile);\n+\tslot->local = NULL;\n \n \tret = move_temp_to_file(tmpfile, filename);\n \tif (ret)\n-- \n1.6.3.1\n"},{"id":"115162","messageId":"7vws7xqazi.fsf@alter.siamese.dyndns.org","threadId":"19051","inReplyTo":"20090531175413.962a55c3.rctay89@gmail.com","subject":"Re: [PATCH v2] http*: cleanup slot->local after fclose","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-31T20:21:53Z","receivedAt":"2009-05-31T20:21:53Z","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> I tested this on top of the first 4 patches in 'rc/http-push' in 'pu',\n> applied on 'maint'. [1] I wonder if this should instead be queued for\n> 'pu' [2], since the issue only occurs there, although, conceivably, it\n> *could* happen without those patches in 'pu'.\n\nThat \"*could*\" leaves me puzzled even more.  Could you clarify?\n\nDo you mean \"earlier, nobody tried to fclose(slot->local) twice, but with\nthe four patches there are codepaths that do so, which triggered the bug\nreported by Clemens\"?  But then the issue couldn't have happened without\nthose patches.\n\nIf existing code always knew when slot->local is and is not valid without\nhaving to rely on its NULLness, and that was the reason why nobody tried\nto fclose(slot->local) twice, then I'd agree that it is a bug waiting to\nhappen, and stuffing NULL to slot->local after the code fclose() it would\nbe a good clean-up.\n"},{"id":"115202","messageId":"be6fef0d0906010652v4fc814f9j631795cbad61be9@mail.gmail.com","threadId":"19051","inReplyTo":"7vws7xqazi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] http*: cleanup slot->local after fclose","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-01T13:52:33Z","receivedAt":"2009-06-01T13:52:33Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Mon, Jun 1, 2009 at 4:21 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> That \"*could*\" leaves me puzzled even more.  Could you clarify?\n>\n> Do you mean \"earlier, nobody tried to fclose(slot->local) twice, but with\n> the four patches there are codepaths that do so, which triggered the bug\n> reported by Clemens\"?  But then the issue couldn't have happened without\n> those patches.\n>\n> If existing code always knew when slot->local is and is not valid without\n> having to rely on its NULLness, and that was the reason why nobody tried\n> to fclose(slot->local) twice, then I'd agree that it is a bug waiting to\n> happen, and stuffing NULL to slot->local after the code fclose() it would\n> be a good clean-up.\n\nthe problem isn't triggered by fclose(slot->local) being done twice,\nbut a ftell() being done on slot->local, which points to a FILE*\nhandle that has already been fclose()'d (that is what the valgrind log\nprovided by Clemens reported). The offending ftell() is found in\nhttp.c::run_active_slot(), and the code there does indeed depend on\nthe NULLness of slot->local (it checks whether slot->local is NULL\nbefore doing the ftell()).\n\nrun_active_slot() is used rather heavily, and users of slot->local\nalways neglect to set it to NULL doing a fclose() on the FILE* handle\nit points to. This statement is true in both 'master', and 'pu'\n('rc/http-push'). Therefore, I said \"*could*\": although the problem\nClemens reported occurs only in 'pu' (or more accurately, the first\nfour patches of 'rc/http-push'), it's not totally impossible for the\nproblem to occur without those four patches.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"115208","messageId":"7vws7wkk4j.fsf@alter.siamese.dyndns.org","threadId":"19051","inReplyTo":"be6fef0d0906010652v4fc814f9j631795cbad61be9@mail.gmail.com","subject":"Re: [PATCH v2] http*: cleanup slot->local after fclose","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-01T16:13:16Z","receivedAt":"2009-06-01T16:13:16Z","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> the problem isn't triggered by fclose(slot->local) being done twice,\n> but a ftell() being done on slot->local, which points to a FILE*\n> handle that has already been fclose()'d (that is what the valgrind log\n> provided by Clemens reported). The offending ftell() is found in\n> http.c::run_active_slot(), and the code there does indeed depend on\n> the NULLness of slot->local (it checks whether slot->local is NULL\n> before doing the ftell()).\n>\n> run_active_slot() is used rather heavily, and users of slot->local\n> always neglect to set it to NULL doing a fclose() on the FILE* handle\n> it points to.\n\nThanks; that clearly indicates that you are fixing an existing bug.\n\nI wish the commit log message explained that from the beginning without\nbeing asked.\n\nSo what would we want to do?\n\nNo patch from your http-push topic is in 'next' yet, and I presume that\nthe same issue exists in 'maint', so I'd say:\n\n (0) queue your slimmed down fix to 'maint' (or rc/maint-http-local-slot-fix\n     that is forked from 'maint', to later merge to 'maint');\n\n (1) create a new rc/http-push topic branch from 'master';\n\n (2) merge (0) into the result of (1);\n \n (3) re-apply the patches queued on the current rc/http-push topic on top\n     of the result of (2).\n\nto rebuild it, perhaps?\n"},{"id":"115243","messageId":"be6fef0d0906020655w64fa7653t908347b3dfc98f7b@mail.gmail.com","threadId":"19051","inReplyTo":"7vws7wkk4j.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] http*: cleanup slot->local after fclose","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-02T13:55:23Z","receivedAt":"2009-06-02T13:55:23Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Tue, Jun 2, 2009 at 12:13 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> I wish the commit log message explained that from the beginning without\n> being asked.\n\nIndeed, I guess I only hinted at the bug being fixed, and in the last\nline, at that.\n\n> So what would we want to do?\n\nYou can go ahead and apply this patch, many thanks. Don't bother with\nre-applying the rc/http-push though; I'll send in the revised patches\nsometime this weekend.\n\n-- \nCheers,\nRay Chuan\n"}]}