{"thread":{"id":"19691","subject":"[PATCH 15/23] http.c: new functions for the http API","startedAt":"2009-06-05T16:05:50Z","lastAt":"2009-06-06T07:21:29Z","messageCount":4,"participants":["Tay Ray Chuan","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":23},"messages":[{"id":"115527","messageId":"20090606000550.17d48ace.rctay89@gmail.com","threadId":"19691","inReplyTo":null,"subject":"[PATCH 15/23] http.c: new functions for the http API","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-05T16:05:50Z","receivedAt":"2009-06-05T16:05:50Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"From:\tMike Hommey <mh@glandium.org>\nSubject: [WIP Patch 03/12] Two new functions for the http API\nDate:\tSun, 18 Jan 2009 09:04:28 +0100\n\nhttp_get_strbuf and http_get_file allow respectively to retrieve contents of\nan URL to a strbuf or an opened file handle.\n\nhttp_error prints out an error message containing the URL and the curl error\n(in curl_errorstr).\n\nSigned-off-by: Mike Hommey <mh@glandium.org>\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n\nI've added an additional slot->local = NULL when cleaning up in\nhttp_request().\n\n http.c |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n http.h |   30 ++++++++++++++++++\n 2 files changed, 134 insertions(+), 0 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 75fce9e..df22180 100644\n--- a/http.c\n+++ b/http.c\n@@ -665,6 +665,110 @@ static char *quote_ref_url(const char *base, const char *ref)\n \treturn strbuf_detach(&buf, NULL);\n }\n\n+/* http_request() targets */\n+#define HTTP_REQUEST_STRBUF\t0\n+#define HTTP_REQUEST_FILE\t1\n+\n+static int http_request(const char *url, void *result, int target, int options)\n+{\n+\tstruct active_request_slot *slot;\n+\tstruct slot_results results;\n+\tstruct curl_slist *headers = NULL;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint ret;\n+\n+\tslot = get_active_slot();\n+\tslot->results = &results;\n+\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n+\n+\tif (result == NULL) {\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);\n+\t} else {\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, result);\n+\n+\t\tif (target == HTTP_REQUEST_FILE) {\n+\t\t\tlong posn = ftell(result);\n+\t\t\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,\n+\t\t\t\t\t fwrite);\n+\t\t\tif (posn > 0) {\n+\t\t\t\tstrbuf_addf(&buf, \"Range: bytes=%ld-\", posn);\n+\t\t\t\theaders = curl_slist_append(headers, buf.buf);\n+\t\t\t\tstrbuf_reset(&buf);\n+\t\t\t}\n+\t\t\tslot->local = result;\n+\t\t} else\n+\t\t\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,\n+\t\t\t\t\t fwrite_buffer);\n+\t}\n+\n+\tstrbuf_addstr(&buf, \"Pragma:\");\n+\tif (options & HTTP_NO_CACHE)\n+\t\tstrbuf_addstr(&buf, \" no-cache\");\n+\n+\theaders = curl_slist_append(headers, buf.buf);\n+\n+\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);\n+\n+\tif (start_active_slot(slot)) {\n+\t\trun_active_slot(slot);\n+\t\tif (results.curl_result == CURLE_OK)\n+\t\t\tret = HTTP_OK;\n+\t\telse if (missing_target(&results))\n+\t\t\tret = HTTP_MISSING_TARGET;\n+\t\telse\n+\t\t\tret = HTTP_ERROR;\n+\t} else {\n+\t\terror(\"Unable to start HTTP request for %s\", url);\n+\t\tret = HTTP_START_FAILED;\n+\t}\n+\n+\tslot->local = NULL;\n+\tcurl_slist_free_all(headers);\n+\tstrbuf_release(&buf);\n+\n+\treturn ret;\n+}\n+\n+int http_get_strbuf(const char *url, struct strbuf *result, int options)\n+{\n+\treturn http_request(url, result, HTTP_REQUEST_STRBUF, options);\n+}\n+\n+int http_get_file(const char *url, const char *filename, int options)\n+{\n+\tint ret;\n+\tstruct strbuf tmpfile = STRBUF_INIT;\n+\tFILE *result;\n+\n+\tstrbuf_addf(&tmpfile, \"%s.temp\", filename);\n+\tresult = fopen(tmpfile.buf, \"a\");\n+\tif (! result) {\n+\t\terror(\"Unable to open local file %s\", tmpfile.buf);\n+\t\tret = HTTP_ERROR;\n+\t\tgoto cleanup;\n+\t}\n+\n+\tret = http_request(url, result, HTTP_REQUEST_FILE, options);\n+\tfclose(result);\n+\n+\tif ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))\n+\t\tret = HTTP_ERROR;\n+cleanup:\n+\tstrbuf_release(&tmpfile);\n+\treturn ret;\n+}\n+\n+int http_error(const char *url, int ret)\n+{\n+\t/* http_request has already handled HTTP_START_FAILED. */\n+\tif (ret != HTTP_START_FAILED)\n+\t\terror(\"%s while accessing %s\\n\", curl_errorstr, url);\n+\n+\treturn ret;\n+}\n+\n int http_fetch_ref(const char *base, struct ref *ref)\n {\n \tchar *url;\ndiff --git a/http.h b/http.h\nindex 1ef7dc1..3d878d5 100644\n--- a/http.h\n+++ b/http.h\n@@ -114,6 +114,36 @@ static inline int missing__target(int code, int result)\n\n #define missing_target(a) missing__target((a)->http_code, (a)->curl_result)\n\n+/* Options for http_request_*() */\n+#define HTTP_NO_CACHE\t\t1\n+\n+/* Return values for http_request_*() */\n+#define HTTP_OK\t\t\t0\n+#define HTTP_MISSING_TARGET\t1\n+#define HTTP_ERROR\t\t2\n+#define HTTP_START_FAILED\t3\n+\n+/*\n+ * Requests an url and stores the result in a strbuf.\n+ *\n+ * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.\n+ */\n+int http_get_strbuf(const char *url, struct strbuf *result, int options);\n+\n+/*\n+ * Downloads an url and stores the result in the given file.\n+ *\n+ * If a previous interrupted download is detected (i.e. a previous temporary\n+ * file is still around) the download is resumed.\n+ */\n+int http_get_file(const char *url, const char *filename, int options);\n+\n+/*\n+ * Prints an error message using error() containing url and curl_errorstr,\n+ * and returns ret.\n+ */\n+int http_error(const char *url, int ret);\n+\n extern int http_fetch_ref(const char *base, struct ref *ref);\n\n #endif /* HTTP_H */\n--\n1.6.3.1\n"},{"id":"115565","messageId":"m3iqja5rvc.fsf@localhost.localdomain","threadId":"19691","inReplyTo":"20090606000550.17d48ace.rctay89@gmail.com","subject":"Re: [PATCH 15/23] http.c: new functions for the http API","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-06-05T20:50:00Z","receivedAt":"2009-06-05T20:50:00Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Tay Ray Chuan <rctay89@gmail.com> writes:\n\n> From:\tMike Hommey <mh@glandium.org>\n> Subject: [WIP Patch 03/12] Two new functions for the http API\n\nSo what is the title of this patch: \n  \"http.c: new functions for the http API\"\nor\n  \"Two new functions for the http API\"\n\nOr perhaps it better be\n  \"http.c: Add http_get_strbuf() and http_get_file() to http API\"\n\n> Date:\tSun, 18 Jan 2009 09:04:28 +0100\n> \n> http_get_strbuf and http_get_file allow respectively to retrieve contents of\n> an URL to a strbuf or an opened file handle.\n> \n> http_error prints out an error message containing the URL and the curl error\n> (in curl_errorstr).\n> \n> Signed-off-by: Mike Hommey <mh@glandium.org>\n> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n> ---\n> \n> I've added an additional slot->local = NULL when cleaning up in\n> http_request().\n> \n>  http.c |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  http.h |   30 ++++++++++++++++++\n>  2 files changed, 134 insertions(+), 0 deletions(-)\n> \n> diff --git a/http.c b/http.c\n> index 75fce9e..df22180 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -665,6 +665,110 @@ static char *quote_ref_url(const char *base, const char *ref)\n>  \treturn strbuf_detach(&buf, NULL);\n>  }\n> \n> +/* http_request() targets */\n> +#define HTTP_REQUEST_STRBUF\t0\n> +#define HTTP_REQUEST_FILE\t1\n> +\n> +static int http_request(const char *url, void *result, int target, int options)\n> +{\n> +\tstruct active_request_slot *slot;\n> +\tstruct slot_results results;\n> +\tstruct curl_slist *headers = NULL;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tint ret;\n> +\n> +\tslot = get_active_slot();\n> +\tslot->results = &results;\n> +\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n> +\n> +\tif (result == NULL) {\n> +\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);\n> +\t} else {\n> +\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n> +\t\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, result);\n> +\n> +\t\tif (target == HTTP_REQUEST_FILE) {\n> +\t\t\tlong posn = ftell(result);\n> +\t\t\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,\n> +\t\t\t\t\t fwrite);\n> +\t\t\tif (posn > 0) {\n> +\t\t\t\tstrbuf_addf(&buf, \"Range: bytes=%ld-\", posn);\n> +\t\t\t\theaders = curl_slist_append(headers, buf.buf);\n> +\t\t\t\tstrbuf_reset(&buf);\n> +\t\t\t}\n> +\t\t\tslot->local = result;\n> +\t\t} else\n> +\t\t\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,\n> +\t\t\t\t\t fwrite_buffer);\n> +\t}\n> +\n> +\tstrbuf_addstr(&buf, \"Pragma:\");\n> +\tif (options & HTTP_NO_CACHE)\n> +\t\tstrbuf_addstr(&buf, \" no-cache\");\n> +\n> +\theaders = curl_slist_append(headers, buf.buf);\n> +\n> +\tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n> +\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);\n> +\n> +\tif (start_active_slot(slot)) {\n> +\t\trun_active_slot(slot);\n> +\t\tif (results.curl_result == CURLE_OK)\n> +\t\t\tret = HTTP_OK;\n> +\t\telse if (missing_target(&results))\n> +\t\t\tret = HTTP_MISSING_TARGET;\n> +\t\telse\n> +\t\t\tret = HTTP_ERROR;\n> +\t} else {\n> +\t\terror(\"Unable to start HTTP request for %s\", url);\n> +\t\tret = HTTP_START_FAILED;\n> +\t}\n> +\n> +\tslot->local = NULL;\n> +\tcurl_slist_free_all(headers);\n> +\tstrbuf_release(&buf);\n> +\n> +\treturn ret;\n> +}\n> +\n> +int http_get_strbuf(const char *url, struct strbuf *result, int options)\n> +{\n> +\treturn http_request(url, result, HTTP_REQUEST_STRBUF, options);\n> +}\n> +\n> +int http_get_file(const char *url, const char *filename, int options)\n> +{\n> +\tint ret;\n> +\tstruct strbuf tmpfile = STRBUF_INIT;\n> +\tFILE *result;\n> +\n> +\tstrbuf_addf(&tmpfile, \"%s.temp\", filename);\n> +\tresult = fopen(tmpfile.buf, \"a\");\n> +\tif (! result) {\n> +\t\terror(\"Unable to open local file %s\", tmpfile.buf);\n> +\t\tret = HTTP_ERROR;\n> +\t\tgoto cleanup;\n> +\t}\n> +\n> +\tret = http_request(url, result, HTTP_REQUEST_FILE, options);\n> +\tfclose(result);\n> +\n> +\tif ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))\n> +\t\tret = HTTP_ERROR;\n> +cleanup:\n> +\tstrbuf_release(&tmpfile);\n> +\treturn ret;\n> +}\n> +\n> +int http_error(const char *url, int ret)\n> +{\n> +\t/* http_request has already handled HTTP_START_FAILED. */\n> +\tif (ret != HTTP_START_FAILED)\n> +\t\terror(\"%s while accessing %s\\n\", curl_errorstr, url);\n> +\n> +\treturn ret;\n> +}\n> +\n>  int http_fetch_ref(const char *base, struct ref *ref)\n>  {\n>  \tchar *url;\n> diff --git a/http.h b/http.h\n> index 1ef7dc1..3d878d5 100644\n> --- a/http.h\n> +++ b/http.h\n> @@ -114,6 +114,36 @@ static inline int missing__target(int code, int result)\n> \n>  #define missing_target(a) missing__target((a)->http_code, (a)->curl_result)\n> \n> +/* Options for http_request_*() */\n> +#define HTTP_NO_CACHE\t\t1\n> +\n> +/* Return values for http_request_*() */\n> +#define HTTP_OK\t\t\t0\n> +#define HTTP_MISSING_TARGET\t1\n> +#define HTTP_ERROR\t\t2\n> +#define HTTP_START_FAILED\t3\n> +\n> +/*\n> + * Requests an url and stores the result in a strbuf.\n> + *\n> + * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.\n> + */\n> +int http_get_strbuf(const char *url, struct strbuf *result, int options);\n> +\n> +/*\n> + * Downloads an url and stores the result in the given file.\n> + *\n> + * If a previous interrupted download is detected (i.e. a previous temporary\n> + * file is still around) the download is resumed.\n> + */\n> +int http_get_file(const char *url, const char *filename, int options);\n> +\n> +/*\n> + * Prints an error message using error() containing url and curl_errorstr,\n> + * and returns ret.\n> + */\n> +int http_error(const char *url, int ret);\n> +\n>  extern int http_fetch_ref(const char *base, struct ref *ref);\n> \n>  #endif /* HTTP_H */\n> --\n> 1.6.3.1\n> \n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"115589","messageId":"be6fef0d0906051808k19a5a4acu29713c5e84fac4d7@mail.gmail.com","threadId":"19691","inReplyTo":"m3iqja5rvc.fsf@localhost.localdomain","subject":"Re: [PATCH 15/23] http.c: new functions for the http API","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-06T01:08:23Z","receivedAt":"2009-06-06T01:08:23Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sat, Jun 6, 2009 at 4:50 AM, Jakub Narebski<jnareb@gmail.com> wrote:\n> So what is the title of this patch:\n>  \"http.c: new functions for the http API\"\n> or\n>  \"Two new functions for the http API\"\n\nSee my reply to patch #19.\n\n> Or perhaps it better be\n>  \"http.c: Add http_get_strbuf() and http_get_file() to http API\"\n\nI also add http_error(). Listing out 1 or 2 functions in the subject\nis ok, but 3 is a stretch, IMO; I think the reader can know the\nfunctions added by reading the commit message.\n\nPerhaps you think an enumerated list of the functions added in the\ncommit message would be preferable?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"115609","messageId":"200906060921.30582.jnareb@gmail.com","threadId":"19691","inReplyTo":"be6fef0d0906051808k19a5a4acu29713c5e84fac4d7@mail.gmail.com","subject":"Re: [PATCH 15/23] http.c: new functions for the http API","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-06-06T07:21:29Z","receivedAt":"2009-06-06T07:21:29Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 6 Jun 2009, Tay Ray Chuan wrote:\n> On Sat, Jun 6, 2009 at 4:50 AM, Jakub Narebski<jnareb@gmail.com> \nwrote:\n\n> > Or perhaps it better be\n> >  \"http.c: Add http_get_strbuf() and http_get_file() to http API\"\n> \n> I also add http_error(). Listing out 1 or 2 functions in the subject\n> is ok, but 3 is a stretch, IMO; I think the reader can know the\n> functions added by reading the commit message.\n\nYou are right. Somehow I missed mentioning http_error() in the commit\nmessage.\n\n> \n> Perhaps you think an enumerated list of the functions added in the\n> commit message would be preferable?\n\nI think the way you did in other commit messages in this thread, with \nenumerated list of added functions (even if in case of this patch they \nare set, rather than sequence), and later description of each function \n(or subset of function) is a better way.\n\n-- \nJakub Narebski\nPoland\n"}]}