Junio C Hamano wrote:
> > + if (http_proxy_authmethod) {
> > + int i;
> > + for (i = 0; i < ARRAY_SIZE(http_proxy_authmethods); i++) {
> > + if (!strcmp(http_proxy_authmethod, http_proxy_authmethods[i].name)) {
> > + curl_easy_setopt(result, CURLOPT_PROXYAUTH,
> > + http_proxy_authmethods[i].curlauth_param);
> > + break;
> > + }
> > + }
> > + if (i == ARRAY_SIZE(http_proxy_authmethods)) {
> > + warning("unsupported proxy authentication method %s: using default",
> > + http_proxy_authmethod);
> > + }
> > + }
> > +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> > + else
> > + curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
> > +#endif
> > +}
>
> This patch should take what 1c2dbf20 (http: support curl < 7.10.7,
> 2015-02-03) wanted to do into account. Having the configuration
> variable or the environment variable defined by itself, while
> running a Git built with old cURL, shouldn't trigger any warning,
> but the entire function should perhaps be ifdefed out or something?
Maybe add a #define LIBCURL_CAN_HANDLE_PROXY_AUTH to clarify this, like we do
with LIBCURL_CAN_HANDLE_AUTH_ANY? If so, should this be a separate patch? With
the current (scattered) version dependencies, it took me a while to realize that
if the function is ifdefed out for LIBCURL_VERSION_NUM < 0x070a07, we don't need
to worry about default behavior in case LIBCURL_CAN_HANDLE_AUTH_ANY is not
defined. (on the other hand, looking at other curl-version-ifdefs, the define
for AUTH_ANY is the exception)
> >> +static void copy_from_env(const char **var, const char *envname)
> >> +{
> >> + const char *val = getenv(envname);
> >> + if (val)
> >> + *var = xstrdup(val);
> >> +}
[...]
> I primarily was
> wishing that its name more clearly conveyed that it sets the
> variable from the environment _only if_ the environment variable
> exists, and otherwise it does not clobber.
How about env_override? Not perfect, but probably better. I don't think
squeezing in more information (maybe_env_override, override_from_env_var) would
help.
> The implementation of the helper seems to assume that the variable
> must not be pointing at a free-able piece of memory when it is
> called
In fact, if http.proxyAuthMethod (btw, I agree with Eric about capitalization)
is set, I do call it with *var pointing to free-able memory. Will fix this.
FWIW, set_from_env() has the same pre-condition, which doesn't seem to be
satisfied in all cases (namely when overwriting variables previously set by
git_config_string()); not to mention missing free()s in http_cleanup.
Otherwise, I'll make the suggested fixes. Thanks.
Cheers,
Knut
--
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Dr. Arno Steitz
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html