Re: [PATCH 2/3] http: try standard proxy env vars when http.proxy config option is not set

On 02/29/2012 10:08 PM, Jeff King wrote:

On Wed, Feb 29, 2012 at 11:46:03AM +0100, Nelson Benitez Leon wrote:

quoted

quoted

Good point. We sometimes follow this order:

  1. git-specific environment variables (i.e., $GIT_HTTP_PROXY, if
     it existed)
  2. git config files (i.e., http.proxy)
  3. generic system environment (i.e., $http_proxy).

So thinking about it that way, the original patch makes more sense.

So, in PATCH 2/3, apart from expanding the commit message.. do we want
to support HTTP_PROXY or only http_proxy ? HTTP_PROXY seems to not be
very used by existent programs, but support it it's only a gentenv call..

If HTTP_PROXY is not in wide use, I don't see a reason to support it.

Ok

And I take back what I said about environment precedence, based on the
discussion. Also, I don't think there is a need to strdup the results of
getenv here, is there? So I think the code you want is just:

  if (!curl_http_proxy)
          curl_http_proxy = getenv("http_proxy");

but curl_http_proxy gets freed in http_cleanup as follows:

free((void *)curl_http_proxy);

Is it ok to free strings returned by getenv() ? I thought nope, so I
used strdup which existent code was already using..

and the justification for the commit message is that we need to know the
proxy value outside of curl, because the next patch will do some
extra processing on the value.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html