{"thread":{"id":"3200","subject":"[PATCH] Fix HTTP request result processing after slot reuse","startedAt":"2006-01-31T19:06:55Z","lastAt":"2006-02-01T01:50:47Z","messageCount":3,"participants":["Nick Hengeveld","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"15316","messageId":"20060131190655.GE3873@reactrix.com","threadId":"3200","inReplyTo":null,"subject":"[PATCH] Fix HTTP request result processing after slot reuse","fromName":"Nick Hengeveld","fromEmail":"nickh@reactrix.com","sentAt":"2006-01-31T19:06:55Z","receivedAt":"2006-01-31T19:06:55Z","isPatch":true,"sender":{"key":"nickh@reactrix.com","avatar":null},"body":"Add a way to store the results of an HTTP request when a slot finishes\nso the results can be processed after the slot has been reused.\n\nSigned-off-by: Nick Hengeveld <nickh@reactrix.com>\n\n\n---\n\n http-fetch.c |   20 ++++++++++++++------\n http.c       |    9 ++++++++-\n http.h       |    7 +++++++\n 3 files changed, 29 insertions(+), 7 deletions(-)\n\necda781d5eec60edc3683a4e7408c99eb93b463c\ndiff --git a/http-fetch.c b/http-fetch.c\nindex 61b2188..92326f9 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -375,6 +375,7 @@ static int fetch_index(struct alt_base *\n \n \tFILE *indexfile;\n \tstruct active_request_slot *slot;\n+\tstatic struct slot_results results;\n \n \tif (has_pack_index(sha1))\n \t\treturn 0;\n@@ -393,6 +394,7 @@ static int fetch_index(struct alt_base *\n \t\t\t     filename);\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@@ -414,7 +416,7 @@ static int fetch_index(struct alt_base *\n \n \tif (start_active_slot(slot)) {\n \t\trun_active_slot(slot);\n-\t\tif (slot->curl_result != CURLE_OK) {\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@@ -616,6 +618,7 @@ static int fetch_indices(struct alt_base\n \tint i = 0;\n \n \tstruct active_request_slot *slot;\n+\tstatic struct slot_results results;\n \n \tif (repo->got_indices)\n \t\treturn 0;\n@@ -632,15 +635,16 @@ static int fetch_indices(struct alt_base\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 (slot->curl_result != CURLE_OK) {\n-\t\t\tif (slot->http_code == 404 ||\n-\t\t\t    slot->curl_result == CURLE_FILE_COULDNT_READ_FILE) {\n+\t\tif (results.curl_result != CURLE_OK) {\n+\t\t\tif (results.http_code == 404 ||\n+\t\t\t    results.curl_result == CURLE_FILE_COULDNT_READ_FILE) {\n \t\t\t\trepo->got_indices = 1;\n \t\t\t\tfree(buffer.buffer);\n \t\t\t\treturn 0;\n@@ -695,6 +699,7 @@ static int fetch_pack(struct alt_base *r\n \tstruct curl_slist *range_header = NULL;\n \n \tstruct active_request_slot *slot;\n+\tstatic struct slot_results results;\n \n \tif (fetch_indices(repo))\n \t\treturn -1;\n@@ -721,6 +726,7 @@ static int fetch_pack(struct alt_base *r\n \t\t\t     filename);\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@@ -742,7 +748,7 @@ static int fetch_pack(struct alt_base *r\n \n \tif (start_active_slot(slot)) {\n \t\trun_active_slot(slot);\n-\t\tif (slot->curl_result != CURLE_OK) {\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@@ -894,6 +900,7 @@ int fetch_ref(char *ref, unsigned char *\n         struct buffer buffer;\n \tchar *base = alt->base;\n \tstruct active_request_slot *slot;\n+\tstatic struct slot_results results;\n         buffer.size = 41;\n         buffer.posn = 0;\n         buffer.buffer = hex;\n@@ -901,13 +908,14 @@ int fetch_ref(char *ref, unsigned char *\n         \n \turl = quote_ref_url(base, ref);\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 (slot->curl_result != CURLE_OK)\n+\t\tif (results.curl_result != CURLE_OK)\n \t\t\treturn error(\"Couldn't get %s for %s\\n%s\",\n \t\t\t\t     url, ref, curl_errorstr);\n \t} else {\ndiff --git a/http.c b/http.c\nindex 75e6717..eefb0f0 100644\n--- a/http.c\n+++ b/http.c\n@@ -335,6 +335,7 @@ struct active_request_slot *get_active_s\n \tactive_requests++;\n \tslot->in_use = 1;\n \tslot->local = NULL;\n+\tslot->results = NULL;\n \tslot->callback_data = NULL;\n \tslot->callback_func = NULL;\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, pragma_header);\n@@ -421,7 +422,13 @@ static void finish_active_slot(struct ac\n         active_requests--;\n         slot->in_use = 0;\n         curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);\n- \n+\n+\t/* Store slot results so they can be read after the slot is reused */\n+\tif (slot->results != NULL) {\n+\t\tslot->results->curl_result = slot->curl_result;\n+\t\tslot->results->http_code = slot->http_code;\n+\t}\n+\n         /* Run callback if appropriate */\n         if (slot->callback_func != NULL) {\n                 slot->callback_func(slot->callback_data);\ndiff --git a/http.h b/http.h\nindex ed4ea33..d6dc9d8 100644\n--- a/http.h\n+++ b/http.h\n@@ -22,6 +22,12 @@\n #define NO_CURL_EASY_DUPHANDLE\n #endif\n \n+struct slot_results\n+{\n+\tCURLcode curl_result;\n+\tlong http_code;\n+};\n+\n struct active_request_slot\n {\n \tCURL *curl;\n@@ -29,6 +35,7 @@ struct active_request_slot\n \tint in_use;\n \tCURLcode curl_result;\n \tlong http_code;\n+\tstruct slot_results *results;\n \tvoid *callback_data;\n \tvoid (*callback_func)(void *data);\n \tstruct active_request_slot *next;\n-- \n1.1.6.g1a5c3-dirty\n"},{"id":"15332","messageId":"7v64o0ulfu.fsf@assigned-by-dhcp.cox.net","threadId":"3200","inReplyTo":"20060131190655.GE3873@reactrix.com","subject":"Re: [PATCH] Fix HTTP request result processing after slot reuse","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-01-31T21:39:01Z","receivedAt":"2006-01-31T21:39:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nick Hengeveld <nickh@reactrix.com> writes:\n\n> Add a way to store the results of an HTTP request when a slot finishes\n> so the results can be processed after the slot has been reused.\n>\n> Signed-off-by: Nick Hengeveld <nickh@reactrix.com>\n\nI do not claim I understand this part of the code well, but it\nfeels awkward to see that a problem caused by a structure being\nreused can be solved by having a pointer from that structure\nthat points at a static location.\n\nThese static variables are probably correct, provided if\nfetch_index, fetch_indices and friends do not recurse into\nthemselves, but it just gives me this funny feeling...\n\nCare to enlighten me, please?\n"},{"id":"15355","messageId":"20060201015047.GF3873@reactrix.com","threadId":"3200","inReplyTo":"7v64o0ulfu.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix HTTP request result processing after slot reuse","fromName":"Nick Hengeveld","fromEmail":"nickh@reactrix.com","sentAt":"2006-02-01T01:50:47Z","receivedAt":"2006-02-01T01:50:47Z","isPatch":true,"sender":{"key":"nickh@reactrix.com","avatar":null},"body":"On Tue, Jan 31, 2006 at 01:39:01PM -0800, Junio C Hamano wrote:\n\n> These static variables are probably correct, provided if\n> fetch_index, fetch_indices and friends do not recurse into\n> themselves, but it just gives me this funny feeling...\n\nIt's true that in the current implementation we either don't recurse\ninto these functions or we explicitly handle cases where we do such as\nfetch_alternates.  However, I've got no argument as to why the\nstructures should be static and can imagine it just causing problems\ndown the road if we were to eg. start downloading multiple packs\nconcurrently.\n\nI'll follow up with a patch.\n\n--\nFor a successful technology, reality must take precedence over public\nrelations, for nature cannot be fooled.\n"}]}