[PATCH 15/23] http.c: new functions for the http API

[PATCH 15/23] http.c: new functions for the http API

[Tay Ray Chuan]

From:	Mike Hommey <mh@glandium.org>
Subject: [WIP Patch 03/12] Two new functions for the http API
Date:	Sun, 18 Jan 2009 09:04:28 +0100

http_get_strbuf and http_get_file allow respectively to retrieve contents of
an URL to a strbuf or an opened file handle.

http_error prints out an error message containing the URL and the curl error
(in curl_errorstr).

Signed-off-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

I've added an additional slot->local = NULL when cleaning up in
http_request().

 http.c |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 http.h |   30 ++++++++++++++++++
 2 files changed, 134 insertions(+), 0 deletions(-)

diff --git a/http.c b/http.c
index 75fce9e..df22180 100644
--- a/http.c
+++ b/http.c
@@ -665,6 +665,110 @@ static char *quote_ref_url(const char *base, const char *ref)
 	return strbuf_detach(&buf, NULL);
 }

+/* http_request() targets */
+#define HTTP_REQUEST_STRBUF	0
+#define HTTP_REQUEST_FILE	1
+
+static int http_request(const char *url, void *result, int target, int options)
+{
+	struct active_request_slot *slot;
+	struct slot_results results;
+	struct curl_slist *headers = NULL;
+	struct strbuf buf = STRBUF_INIT;
+	int ret;
+
+	slot = get_active_slot();
+	slot->results = &results;
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
+
+	if (result == NULL) {
+		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
+	} else {
+		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+		curl_easy_setopt(slot->curl, CURLOPT_FILE, result);
+
+		if (target == HTTP_REQUEST_FILE) {
+			long posn = ftell(result);
+			curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
+					 fwrite);
+			if (posn > 0) {
+				strbuf_addf(&buf, "Range: bytes=%ld-", posn);
+				headers = curl_slist_append(headers, buf.buf);
+				strbuf_reset(&buf);
+			}
+			slot->local = result;
+		} else
+			curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
+					 fwrite_buffer);
+	}
+
+	strbuf_addstr(&buf, "Pragma:");
+	if (options & HTTP_NO_CACHE)
+		strbuf_addstr(&buf, " no-cache");
+
+	headers = curl_slist_append(headers, buf.buf);
+
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
+
+	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
+			ret = HTTP_ERROR;
+	} else {
+		error("Unable to start HTTP request for %s", url);
+		ret = HTTP_START_FAILED;
+	}
+
+	slot->local = NULL;
+	curl_slist_free_all(headers);
+	strbuf_release(&buf);
+
+	return ret;
+}
+
+int http_get_strbuf(const char *url, struct strbuf *result, int options)
+{
+	return http_request(url, result, HTTP_REQUEST_STRBUF, options);
+}
+
+int http_get_file(const char *url, const char *filename, int options)
+{
+	int ret;
+	struct strbuf tmpfile = STRBUF_INIT;
+	FILE *result;
+
+	strbuf_addf(&tmpfile, "%s.temp", filename);
+	result = fopen(tmpfile.buf, "a");
+	if (! result) {
+		error("Unable to open local file %s", tmpfile.buf);
+		ret = HTTP_ERROR;
+		goto cleanup;
+	}
+
+	ret = http_request(url, result, HTTP_REQUEST_FILE, options);
+	fclose(result);
+
+	if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))
+		ret = HTTP_ERROR;
+cleanup:
+	strbuf_release(&tmpfile);
+	return ret;
+}
+
+int http_error(const char *url, int ret)
+{
+	/* http_request has already handled HTTP_START_FAILED. */
+	if (ret != HTTP_START_FAILED)
+		error("%s while accessing %s\n", curl_errorstr, url);
+
+	return ret;
+}
+
 int http_fetch_ref(const char *base, struct ref *ref)
 {
 	char *url;
diff --git a/http.h b/http.h
index 1ef7dc1..3d878d5 100644
--- a/http.h
+++ b/http.h
@@ -114,6 +114,36 @@ static inline int missing__target(int code, int result)

 #define missing_target(a) missing__target((a)->http_code, (a)->curl_result)

+/* Options for http_request_*() */
+#define HTTP_NO_CACHE		1
+
+/* Return values for http_request_*() */
+#define HTTP_OK			0
+#define HTTP_MISSING_TARGET	1
+#define HTTP_ERROR		2
+#define HTTP_START_FAILED	3
+
+/*
+ * Requests an url and stores the result in a strbuf.
+ *
+ * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.
+ */
+int http_get_strbuf(const char *url, struct strbuf *result, int options);
+
+/*
+ * Downloads an url and stores the result in the given file.
+ *
+ * If a previous interrupted download is detected (i.e. a previous temporary
+ * file is still around) the download is resumed.
+ */
+int http_get_file(const char *url, const char *filename, int options);
+
+/*
+ * Prints an error message using error() containing url and curl_errorstr,
+ * and returns ret.
+ */
+int http_error(const char *url, int ret);
+
 extern int http_fetch_ref(const char *base, struct ref *ref);

 #endif /* HTTP_H */
--
1.6.3.1

Re: [PATCH 15/23] http.c: new functions for the http API

[Jakub Narebski]

Tay Ray Chuan <rctay89@gmail.com> writes:

> From:	Mike Hommey <mh@glandium.org>
> Subject: [WIP Patch 03/12] Two new functions for the http API

So what is the title of this patch: 
  "http.c: new functions for the http API"
or
  "Two new functions for the http API"

Or perhaps it better be
  "http.c: Add http_get_strbuf() and http_get_file() to http API"

> Date:	Sun, 18 Jan 2009 09:04:28 +0100
> 
> http_get_strbuf and http_get_file allow respectively to retrieve contents of
> an URL to a strbuf or an opened file handle.
> 
> http_error prints out an error message containing the URL and the curl error
> (in curl_errorstr).
> 
> Signed-off-by: Mike Hommey <mh@glandium.org>
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
> 
> I've added an additional slot->local = NULL when cleaning up in
> http_request().
> 
>  http.c |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  http.h |   30 ++++++++++++++++++
>  2 files changed, 134 insertions(+), 0 deletions(-)
> 
> diff --git a/http.c b/http.c
> index 75fce9e..df22180 100644
> --- a/http.c
> +++ b/http.c
> @@ -665,6 +665,110 @@ static char *quote_ref_url(const char *base, const char *ref)
>  	return strbuf_detach(&buf, NULL);
>  }
> 
> +/* http_request() targets */
> +#define HTTP_REQUEST_STRBUF	0
> +#define HTTP_REQUEST_FILE	1
> +
> +static int http_request(const char *url, void *result, int target, int options)
> +{
> +	struct active_request_slot *slot;
> +	struct slot_results results;
> +	struct curl_slist *headers = NULL;
> +	struct strbuf buf = STRBUF_INIT;
> +	int ret;
> +
> +	slot = get_active_slot();
> +	slot->results = &results;
> +	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
> +
> +	if (result == NULL) {
> +		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
> +	} else {
> +		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
> +		curl_easy_setopt(slot->curl, CURLOPT_FILE, result);
> +
> +		if (target == HTTP_REQUEST_FILE) {
> +			long posn = ftell(result);
> +			curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
> +					 fwrite);
> +			if (posn > 0) {
> +				strbuf_addf(&buf, "Range: bytes=%ld-", posn);
> +				headers = curl_slist_append(headers, buf.buf);
> +				strbuf_reset(&buf);
> +			}
> +			slot->local = result;
> +		} else
> +			curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
> +					 fwrite_buffer);
> +	}
> +
> +	strbuf_addstr(&buf, "Pragma:");
> +	if (options & HTTP_NO_CACHE)
> +		strbuf_addstr(&buf, " no-cache");
> +
> +	headers = curl_slist_append(headers, buf.buf);
> +
> +	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> +	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
> +
> +	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
> +			ret = HTTP_ERROR;
> +	} else {
> +		error("Unable to start HTTP request for %s", url);
> +		ret = HTTP_START_FAILED;
> +	}
> +
> +	slot->local = NULL;
> +	curl_slist_free_all(headers);
> +	strbuf_release(&buf);
> +
> +	return ret;
> +}
> +
> +int http_get_strbuf(const char *url, struct strbuf *result, int options)
> +{
> +	return http_request(url, result, HTTP_REQUEST_STRBUF, options);
> +}
> +
> +int http_get_file(const char *url, const char *filename, int options)
> +{
> +	int ret;
> +	struct strbuf tmpfile = STRBUF_INIT;
> +	FILE *result;
> +
> +	strbuf_addf(&tmpfile, "%s.temp", filename);
> +	result = fopen(tmpfile.buf, "a");
> +	if (! result) {
> +		error("Unable to open local file %s", tmpfile.buf);
> +		ret = HTTP_ERROR;
> +		goto cleanup;
> +	}
> +
> +	ret = http_request(url, result, HTTP_REQUEST_FILE, options);
> +	fclose(result);
> +
> +	if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))
> +		ret = HTTP_ERROR;
> +cleanup:
> +	strbuf_release(&tmpfile);
> +	return ret;
> +}
> +
> +int http_error(const char *url, int ret)
> +{
> +	/* http_request has already handled HTTP_START_FAILED. */
> +	if (ret != HTTP_START_FAILED)
> +		error("%s while accessing %s\n", curl_errorstr, url);
> +
> +	return ret;
> +}
> +
>  int http_fetch_ref(const char *base, struct ref *ref)
>  {
>  	char *url;
> diff --git a/http.h b/http.h
> index 1ef7dc1..3d878d5 100644
> --- a/http.h
> +++ b/http.h
> @@ -114,6 +114,36 @@ static inline int missing__target(int code, int result)
> 
>  #define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
> 
> +/* Options for http_request_*() */
> +#define HTTP_NO_CACHE		1
> +
> +/* Return values for http_request_*() */
> +#define HTTP_OK			0
> +#define HTTP_MISSING_TARGET	1
> +#define HTTP_ERROR		2
> +#define HTTP_START_FAILED	3
> +
> +/*
> + * Requests an url and stores the result in a strbuf.
> + *
> + * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.
> + */
> +int http_get_strbuf(const char *url, struct strbuf *result, int options);
> +
> +/*
> + * Downloads an url and stores the result in the given file.
> + *
> + * If a previous interrupted download is detected (i.e. a previous temporary
> + * file is still around) the download is resumed.
> + */
> +int http_get_file(const char *url, const char *filename, int options);
> +
> +/*
> + * Prints an error message using error() containing url and curl_errorstr,
> + * and returns ret.
> + */
> +int http_error(const char *url, int ret);
> +
>  extern int http_fetch_ref(const char *base, struct ref *ref);
> 
>  #endif /* HTTP_H */
> --
> 1.6.3.1
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH 15/23] http.c: new functions for the http API

[Tay Ray Chuan]

Hi,

On Sat, Jun 6, 2009 at 4:50 AM, Jakub Narebski<jnareb@gmail.com> wrote:
> So what is the title of this patch:
>  "http.c: new functions for the http API"
> or
>  "Two new functions for the http API"

See my reply to patch #19.

> Or perhaps it better be
>  "http.c: Add http_get_strbuf() and http_get_file() to http API"

I also add http_error(). Listing out 1 or 2 functions in the subject
is ok, but 3 is a stretch, IMO; I think the reader can know the
functions added by reading the commit message.

Perhaps you think an enumerated list of the functions added in the
commit message would be preferable?

-- 
Cheers,
Ray Chuan

Re: [PATCH 15/23] http.c: new functions for the http API

[Jakub Narebski]

On Sat, 6 Jun 2009, Tay Ray Chuan wrote:
> On Sat, Jun 6, 2009 at 4:50 AM, Jakub Narebski<jnareb@gmail.com> 
wrote:

> > Or perhaps it better be
> >  "http.c: Add http_get_strbuf() and http_get_file() to http API"
> 
> I also add http_error(). Listing out 1 or 2 functions in the subject
> is ok, but 3 is a stretch, IMO; I think the reader can know the
> functions added by reading the commit message.

You are right. Somehow I missed mentioning http_error() in the commit
message.

> 
> Perhaps you think an enumerated list of the functions added in the
> commit message would be preferable?

I think the way you did in other commit messages in this thread, with 
enumerated list of added functions (even if in case of this patch they 
are set, rather than sequence), and later description of each function 
(or subset of function) is a better way.

-- 
Jakub Narebski
Poland

[PATCH 15/23] http.c: new functions for the http API

[Tay Ray Chuan]

From:	Mike Hommey <mh@glandium.org>

The new functions added are:
 - http_request() (internal function)
 - http_get_strbuf()
 - http_get_file()
 - http_error()

http_get_strbuf and http_get_file allow respectively to retrieve contents of
an URL to a strbuf or an opened file handle.

http_error prints out an error message containing the URL and the curl error
(in curl_errorstr).

Signed-off-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

This patch is based on Mike's earlier patch:

Subject: [WIP Patch 03/12] Two new functions for the http API
Date:	Sun, 18 Jan 2009 09:04:28 +0100

I've added a new return status code for http_request,
HTTP_START_FAILED, since some callers need to know when
start_active_slot() fails.

http_request() will emit an error message when start_active_slot()
fails; for other cases, no error message is emitted.

I've added http_error(). (see description above)

I've added the list of new functions to the commit message body, as
suggested by Jakub.

 http.c |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 http.h |   30 ++++++++++++++++++
 2 files changed, 134 insertions(+), 0 deletions(-)

diff --git a/http.c b/http.c
index 75fce9e..df22180 100644
--- a/http.c
+++ b/http.c
@@ -665,6 +665,110 @@ static char *quote_ref_url(const char *base, const char *ref)
 	return strbuf_detach(&buf, NULL);
 }

+/* http_request() targets */
+#define HTTP_REQUEST_STRBUF	0
+#define HTTP_REQUEST_FILE	1
+
+static int http_request(const char *url, void *result, int target, int options)
+{
+	struct active_request_slot *slot;
+	struct slot_results results;
+	struct curl_slist *headers = NULL;
+	struct strbuf buf = STRBUF_INIT;
+	int ret;
+
+	slot = get_active_slot();
+	slot->results = &results;
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
+
+	if (result == NULL) {
+		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
+	} else {
+		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+		curl_easy_setopt(slot->curl, CURLOPT_FILE, result);
+
+		if (target == HTTP_REQUEST_FILE) {
+			long posn = ftell(result);
+			curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
+					 fwrite);
+			if (posn > 0) {
+				strbuf_addf(&buf, "Range: bytes=%ld-", posn);
+				headers = curl_slist_append(headers, buf.buf);
+				strbuf_reset(&buf);
+			}
+			slot->local = result;
+		} else
+			curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
+					 fwrite_buffer);
+	}
+
+	strbuf_addstr(&buf, "Pragma:");
+	if (options & HTTP_NO_CACHE)
+		strbuf_addstr(&buf, " no-cache");
+
+	headers = curl_slist_append(headers, buf.buf);
+
+	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
+
+	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
+			ret = HTTP_ERROR;
+	} else {
+		error("Unable to start HTTP request for %s", url);
+		ret = HTTP_START_FAILED;
+	}
+
+	slot->local = NULL;
+	curl_slist_free_all(headers);
+	strbuf_release(&buf);
+
+	return ret;
+}
+
+int http_get_strbuf(const char *url, struct strbuf *result, int options)
+{
+	return http_request(url, result, HTTP_REQUEST_STRBUF, options);
+}
+
+int http_get_file(const char *url, const char *filename, int options)
+{
+	int ret;
+	struct strbuf tmpfile = STRBUF_INIT;
+	FILE *result;
+
+	strbuf_addf(&tmpfile, "%s.temp", filename);
+	result = fopen(tmpfile.buf, "a");
+	if (! result) {
+		error("Unable to open local file %s", tmpfile.buf);
+		ret = HTTP_ERROR;
+		goto cleanup;
+	}
+
+	ret = http_request(url, result, HTTP_REQUEST_FILE, options);
+	fclose(result);
+
+	if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))
+		ret = HTTP_ERROR;
+cleanup:
+	strbuf_release(&tmpfile);
+	return ret;
+}
+
+int http_error(const char *url, int ret)
+{
+	/* http_request has already handled HTTP_START_FAILED. */
+	if (ret != HTTP_START_FAILED)
+		error("%s while accessing %s\n", curl_errorstr, url);
+
+	return ret;
+}
+
 int http_fetch_ref(const char *base, struct ref *ref)
 {
 	char *url;
diff --git a/http.h b/http.h
index 1ef7dc1..3d878d5 100644
--- a/http.h
+++ b/http.h
@@ -114,6 +114,36 @@ static inline int missing__target(int code, int result)

 #define missing_target(a) missing__target((a)->http_code, (a)->curl_result)

+/* Options for http_request_*() */
+#define HTTP_NO_CACHE		1
+
+/* Return values for http_request_*() */
+#define HTTP_OK			0
+#define HTTP_MISSING_TARGET	1
+#define HTTP_ERROR		2
+#define HTTP_START_FAILED	3
+
+/*
+ * Requests an url and stores the result in a strbuf.
+ *
+ * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.
+ */
+int http_get_strbuf(const char *url, struct strbuf *result, int options);
+
+/*
+ * Downloads an url and stores the result in the given file.
+ *
+ * If a previous interrupted download is detected (i.e. a previous temporary
+ * file is still around) the download is resumed.
+ */
+int http_get_file(const char *url, const char *filename, int options);
+
+/*
+ * Prints an error message using error() containing url and curl_errorstr,
+ * and returns ret.
+ */
+int http_error(const char *url, int ret);
+
 extern int http_fetch_ref(const char *base, struct ref *ref);

 #endif /* HTTP_H */
--
1.6.3.1