Re: [PATCH 7/8] http: factor out http error code handling

[Junio C Hamano]

Jeff King p...@peff.net writes:

 Most of our http requests go through the http_request()
 interface, which does some nice post-processing on the
 results. In particular, it handles prompting for missing
 credentials as well as approving and rejecting valid or
 invalid credentials. Unfortunately, it only handles GET
 requests. Making it handle POSTs would be quite complex, so
 let's pull result handling code into its own function so
 that it can be reused from the POST code paths.

 Signed-off-by: Jeff King p...@peff.net
 ---
  http.c | 51 ---
  http.h |  1 +
  2 files changed, 29 insertions(+), 23 deletions(-)

 diff --git a/http.c b/http.c
 index b61ac85..6793137 100644
 --- a/http.c
 +++ b/http.c
 @@ -745,6 +745,33 @@ char *get_remote_object_url(const char *url, const char 
 *hex,
   return strbuf_detach(buf, NULL);
  }
  
 +int handle_curl_result(struct active_request_slot *slot)
 +{
 + struct slot_results *results = slot-results;
 +
 + if (results-curl_result == CURLE_OK) {
 + credential_approve(http_auth);
 + return HTTP_OK;
 + } else if (missing_target(results))
 +...
 + return HTTP_ERROR;
 + }
 +}
 +
 @@ -820,9 +828,6 @@ static int http_request(const char *url, void *result, 
 int target, int options)
   curl_slist_free_all(headers);
   strbuf_release(buf);
  
 - if (ret == HTTP_OK)
 - credential_approve(http_auth);

OK, now this is part of handle_curl_result() so the caller does not
have to worry about it, which is nice ;-)

   return ret;
  }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/8] http: factor out http error code handling

[Jeff King]

Most of our http requests go through the http_request()
interface, which does some nice post-processing on the
results. In particular, it handles prompting for missing
credentials as well as approving and rejecting valid or
invalid credentials. Unfortunately, it only handles GET
requests. Making it handle POSTs would be quite complex, so
let's pull result handling code into its own function so
that it can be reused from the POST code paths.

Signed-off-by: Jeff King p...@peff.net
---
 http.c | 51 ---
 http.h |  1 +
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/http.c b/http.c
index b61ac85..6793137 100644
--- a/http.c
+++ b/http.c
@@ -745,6 +745,33 @@ char *get_remote_object_url(const char *url, const char 
*hex,
return strbuf_detach(buf, NULL);
 }
 
+int handle_curl_result(struct active_request_slot *slot)
+{
+   struct slot_results *results = slot-results;
+
+   if (results-curl_result == CURLE_OK) {
+   credential_approve(http_auth);
+   return HTTP_OK;
+   } else if (missing_target(results))
+   return HTTP_MISSING_TARGET;
+   else if (results-http_code == 401) {
+   if (http_auth.username  http_auth.password) {
+   credential_reject(http_auth);
+   return HTTP_NOAUTH;
+   } else {
+   credential_fill(http_auth);
+   init_curl_http_auth(slot-curl);
+   return HTTP_REAUTH;
+   }
+   } else {
+   if (!curl_errorstr[0])
+   strlcpy(curl_errorstr,
+   curl_easy_strerror(results-curl_result),
+   sizeof(curl_errorstr));
+   return HTTP_ERROR;
+   }
+}
+
 /* http_request() targets */
 #define HTTP_REQUEST_STRBUF0
 #define HTTP_REQUEST_FILE  1
@@ -792,26 +819,7 @@ static int http_request(const char *url, void *result, int 
target, int options)
 
if (start_active_slot(slot)) {
run_active_slot(slot);
-   if (results.curl_result == CURLE_OK)
-   ret = HTTP_OK;
-   else if (missing_target(results))
-   ret = HTTP_MISSING_TARGET;
-   else if (results.http_code == 401) {
-   if (http_auth.username  http_auth.password) {
-   credential_reject(http_auth);
-   ret = HTTP_NOAUTH;
-   } else {
-   credential_fill(http_auth);
-   init_curl_http_auth(slot-curl);
-   ret = HTTP_REAUTH;
-   }
-   } else {
-   if (!curl_errorstr[0])
-   strlcpy(curl_errorstr,
-   curl_easy_strerror(results.curl_result),
-   sizeof(curl_errorstr));
-   ret = HTTP_ERROR;
-   }
+   ret = handle_curl_result(slot);
} else {
error(Unable to start HTTP request for %s, url);
ret = HTTP_START_FAILED;
@@ -820,9 +828,6 @@ static int http_request(const char *url, void *result, int 
target, int options)
curl_slist_free_all(headers);
strbuf_release(buf);
 
-   if (ret == HTTP_OK)
-   credential_approve(http_auth);
-
return ret;
 }
 
diff --git a/http.h b/http.h
index 915c286..12de255 100644
--- a/http.h
+++ b/http.h
@@ -78,6 +78,7 @@ extern int start_active_slot(struct active_request_slot 
*slot);
 extern void run_active_slot(struct active_request_slot *slot);
 extern void finish_active_slot(struct active_request_slot *slot);
 extern void finish_all_active_slots(void);
+extern int handle_curl_result(struct active_request_slot *slot);
 
 #ifdef USE_CURL_MULTI
 extern void fill_active_slots(void);
-- 
1.7.11.5.10.g3c8125b

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html