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

[Tay Ray Chuan <rctay89@gmail.com>]

On Thu, Mar 31, 2011 at 9:38 AM, Dan McGee <dpmcgee@gmail.com> wrote:
> 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.

> --- 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