Re: [PATCH v3 3/4] http: handle proxy proactive authentication

[Jeff King <peff@peff.net>]

On Tue, Mar 06, 2012 at 05:09:47AM -0500, Jeff King wrote:

> > Ok, will remove it, I copy/paste it from the http code and I must admit
> > I didn't understand why this was needed.
> 
> Ah.  I grepped for the spot you copied. The cast is to get rid of the
> "const" on curl_http_proxy. But if it's a pointer to allocated memory,
> it should not be declared const in the first place. Unfortunately,
> fixing this means casting in the call to git_config_string (which for
> some reason takes a pointer-to-const-pointer, even though the value it
> puts in will always be allocated by xstrdup). Or fixing
> git_config_string, but that cascades to require fixing in lots of other
> places. Ugh.

I did a little more looking into this. The situation is annoyingly
complex, because some callers really do have const strings. They do
things like this:

  static const char *prune_expire = "2.weeks.ago";
  [...]
  git_config_string(&prune_expire, var, value);

And that one should be const, because the string literal really is
const. Sometimes it is converted into an allocated value, but we
effectively treat it as const.

And then you have things like curl_http_proxy, which are never actually
const, but always allocated strings (that get freed during cleanup).
Those ones should probably not be const.

So I don't think there's an easy solution, and the least evil thing is
probably to just keep the cast on free, as you did (although I think if
you convert to using CURLOPT_PROXYUSERPWD, you won't be rewriting
curl_http_proxy anymore, and thus your free() call will go away).

-Peff