From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "Yi, EungJun" <semtlenori@gmail.com>
Subject: [PATCH 6/9] http: simplify http_error helper function
Date: Fri, 5 Apr 2013 18:21:34 -0400 [thread overview]
Message-ID: <20130405222134.GF22163@sigill.intra.peff.net> (raw)
In-Reply-To: <20130405221331.GA21209@sigill.intra.peff.net>
This helper function should really be a one-liner that
prints an error message, but it has ended up unnecessarily
complicated:
1. We call error() directly when we fail to start the curl
request, so we must later avoid printing a duplicate
error in http_error().
It would be much simpler in this case to just stuff the
error message into our usual curl_errorstr buffer
rather than printing it ourselves. This means that
http_error does not even have to care about curl's exit
value (the interesting part is in the errorstr buffer
already).
2. We return the "ret" value passed in to us, but none of
the callers actually cares about our return value. We
can just drop this entirely.
Signed-off-by: Jeff King <peff@peff.net>
---
http-push.c | 2 +-
http.c | 11 ++++-------
http.h | 5 ++---
remote-curl.c | 2 +-
4 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/http-push.c b/http-push.c
index bd66f6a..439a555 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1551,7 +1551,7 @@ static int remote_exists(const char *path)
ret = 0;
break;
case HTTP_ERROR:
- http_error(url, HTTP_ERROR);
+ http_error(url);
default:
ret = -1;
}
diff --git a/http.c b/http.c
index 45cc7c7..5e6f67d 100644
--- a/http.c
+++ b/http.c
@@ -857,7 +857,8 @@ static int http_request(const char *url, struct strbuf *type,
run_active_slot(slot);
ret = handle_curl_result(&results);
} else {
- error("Unable to start HTTP request for %s", url);
+ snprintf(curl_errorstr, sizeof(curl_errorstr),
+ "failed to start HTTP request");
ret = HTTP_START_FAILED;
}
@@ -940,13 +941,9 @@ int http_error(const char *url, int ret)
return ret;
}
-int http_error(const char *url, int ret)
+void http_error(const char *url)
{
- /* http_request has already handled HTTP_START_FAILED. */
- if (ret != HTTP_START_FAILED)
- error("%s while accessing %s", curl_errorstr, url);
-
- return ret;
+ error("%s while accessing %s", curl_errorstr, url);
}
int http_fetch_ref(const char *base, struct ref *ref)
diff --git a/http.h b/http.h
index 0fe54f4..fa65128 100644
--- a/http.h
+++ b/http.h
@@ -136,10 +136,9 @@ int http_get_strbuf(const char *url, struct strbuf *content_type, struct strbuf
int http_get_strbuf(const char *url, struct strbuf *content_type, struct strbuf *result, int options);
/*
- * Prints an error message using error() containing url and curl_errorstr,
- * and returns ret.
+ * Prints an error message using error() containing url and curl_errorstr.
*/
-int http_error(const char *url, int ret);
+void http_error(const char *url);
extern int http_fetch_ref(const char *base, struct ref *ref);
diff --git a/remote-curl.c b/remote-curl.c
index 6c6714b..9abe4b7 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -216,7 +216,7 @@ static struct discovery* discover_refs(const char *service, int for_push)
die("Authentication failed for '%s'", url);
default:
show_http_message(&type, &buffer);
- http_error(url, http_ret);
+ http_error(url);
die("HTTP request failed");
}
--
1.8.2.rc0.33.gd915649
next prev parent reply other threads:[~2013-04-06 16:57 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-05 22:13 [RFC/PATCH 0/9] friendlier http error messages Jeff King
2013-04-05 22:14 ` [PATCH 1/9] http: add HTTP_KEEP_ERROR option Jeff King
2013-04-05 22:17 ` [PATCH 2/9] remote-curl: show server content on http errors Jeff King
2013-04-05 22:17 ` [PATCH 3/9] remote-curl: let servers override http 404 advice Jeff King
2013-04-05 22:20 ` [PATCH 4/9] remote-curl: always show friendlier 404 message Jeff King
2013-04-05 22:21 ` [PATCH 5/9] remote-curl: consistently report repo url for http errors Jeff King
2013-04-05 22:21 ` Jeff King [this message]
2013-04-05 22:22 ` [PATCH 7/9] http: re-word http error message Jeff King
2013-04-05 22:22 ` [PATCH 8/9] remote-curl: die directly with http error messages Jeff King
2013-04-05 22:22 ` [PATCH 9/9] http: drop http_error function 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130405222134.GF22163@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=semtlenori@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.
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 NNTP newsgroup(s).