Re: [PATCH 2/2] http-push: refactor curl_easy_setup madness

From: Tay Ray Chuan <rctay89@gmail.com>
Date: Apr 16, 2011, 18:24 UTC
Message-ID: <BANLkTinv4M7biQ_Wd3rWYbGLSwgGBmhbUA@mail.gmail.com>
In-Reply-To: <1301535531-1244-2-git-send-email-dpmcgee@gmail.com>

On Thu, Mar 31, 2011 at 9:38 AM, Dan McGee <dpmcgee@gmail.com> wrote:

Show 7 quoted lines

> We were doing (nearly) the same thing all over the place, in slightly
> different orders, different variable names, etc. Refactor most calls
> into two helper functions, one for GET and one for everything else, that
> do the heavy lifting leaving most callsites a lot cleaner in the
> process.
>
> Signed-off-by: Dan McGee <dpmcgee@gmail.com>

Nice work.

Perhaps you should mention in the commit message that the setting of CURLOPT_PUT at the callsites of curl_setup_http() which previously didn't do it (eg. locking_available(), remote_ls()) is ok, since that option is deprecated in place of, and has the same effect as, CURLOPT_UPLOAD.

Show 11 quoted lines

> --- a/http-push.c
> +++ b/http-push.c
> @@ -169,7 +169,7 @@ enum dav_header_flag {
>        DAV_HEADER_TIMEOUT = (1u << 2)
>  };
>
> -static char *xml_entities(char *s)
> +static char *xml_entities(const char *s)
>  {
>        struct strbuf buf = STRBUF_INIT;
>        while (*s) {

Perhaps the addition of "const", and elsewhere in this patch, should be placed in a separate patch.

-- Cheers, Ray Chuan