git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Brad Hein <linuxbrad@gmail.com>, git@vger.kernel.org
Subject: [PATCH 1/2] remote-curl: do not call run_slot repeatedly
Date: Fri, 12 Oct 2012 03:35:33 -0400	[thread overview]
Message-ID: <20121012073533.GA16441@sigill.intra.peff.net> (raw)
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

  reply	other threads:[~2012-10-12  7:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAJa+X0OkzAX9E2SnDmU=on0yzzVZ9OMa2dJZgKMK=gQu2Rhf_Q@mail.gmail.com>
2012-10-12  4:58 ` git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0 Brad Hein
2012-10-12  6:22   ` [PATCH] http: fix segfault in handle_curl_result Jeff King
2012-10-12  7:30     ` Jeff King
2012-10-12  7:35       ` Jeff King [this message]
2012-10-12  7:35       ` [PATCH 2/2] http: do not set up curl auth after a 401 Jeff King
2012-10-12 13:39         ` Brad Hein
2012-10-12 16:46     ` [PATCH] http: fix segfault in handle_curl_result Junio C Hamano
2012-10-12 16:29   ` git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0 Erik Faye-Lund
2012-10-12 16:34     ` Erik Faye-Lund
2012-10-12 17:12       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121012073533.GA16441@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=linuxbrad@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).