git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] remote-curl: do not call run_slot repeatedly

From
Jeff King <peff@peff.net>
Date
Oct 12, 2012, 07:35 UTC
Message-ID
<20121012073533.GA16441@sigill.intra.peff.net>
In-Reply-To
<20121012073053.GC17026@sigill.intra.peff.net>

Commit b81401c (http: prompt for credentials on failed POST) taught post_rpc to call run_slot in a loop in order to retry a request after asking the user for credentials. However, after a call to run_slot we will have called finish_active_slot. This means we have released the slot, and we should no longer look at it.

As it happens, this does not cause any bugs in the current code, since we know that we are not using curl_multi in this code path, and therefore nobody will have taken over our slot in the meantime. However, it is good form to actually call get_active_slot again. It also future proofs us against changes in the http code.

We can do this by jumping back to a retry label at the top of our function. We just need to reorder a few setup lines that should not be repeated; everything else within the loop is either idempotent, needs to be repeated, or in a path we do not follow (e.g., we do not even try when large_request is set, because we don't know how much data we might have streamed from our helper program).

Signed-off-by: Jeff King <peff@peff.net>
---
Without this future-proofing, the next patch causes test
failures.

If the goto is too ugly, we can pull it out into a loop (it just makes the patch harder to read, because the meat of the function gets indented further).

 remote-curl.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 6054e47..4281262 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -444,6 +444,11 @@ static int post_rpc(struct rpc_state *rpc)
 			return -1;
 	}
 
+	headers = curl_slist_append(headers, rpc->hdr_content_type);
+	headers = curl_slist_append(headers, rpc->hdr_accept);
+	headers = curl_slist_append(headers, "Expect:");
+
+retry:
 	slot = get_active_slot();
 
 	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
@@ -451,10 +456,6 @@ static int post_rpc(struct rpc_state *rpc)
 	curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
 
-	headers = curl_slist_append(headers, rpc->hdr_content_type);
-	headers = curl_slist_append(headers, rpc->hdr_accept);
-	headers = curl_slist_append(headers, "Expect:");
-
 	if (large_request) {
 		/* The request body is large and the size cannot be predicted.
 		 * We must use chunked encoding to send it.
@@ -528,9 +529,9 @@ static int post_rpc(struct rpc_state *rpc)
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
 
-	do {
-		err = run_slot(slot);
-	} while (err == HTTP_REAUTH && !large_request && !use_gzip);
+	err = run_slot(slot);
+	if (err == HTTP_REAUTH && !large_request && !use_gzip)
+		goto retry;
 	if (err != HTTP_OK)
 		err = -1;
 
-- 
1.8.0.rc2.3.g303f317
Previous: Jeff KingNext: Jeff King
Message 4 of 10 in “git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0”
  1. Brad HeinOct 12, 2012
  2. http: fix segfault in handle_curl_resultJeff King, Oct 12, 2012
  3. Jeff KingOct 12, 2012
  4. 1/2 remote-curl: do not call run_slot repeatedlyJeff King, Oct 12, 2012
  5. 2/2 http: do not set up curl auth after a 401Jeff King, Oct 12, 2012
  6. Brad HeinOct 12, 2012
  7. Junio C HamanoOct 12, 2012
  8. Erik Faye-LundOct 12, 2012
  9. Erik Faye-LundOct 12, 2012
  10. Jeff KingOct 12, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.