Hi Peff,

On Mon, 25 Apr 2016, Jeff King wrote:

> On Mon, Apr 25, 2016 at 03:13:08PM +0200, Johannes Schindelin wrote:
> 
> > diff --git a/http.c b/http.c
> > index 4304b80..02d7147 100644
> > --- a/http.c
> > +++ b/http.c
> > @@ -114,6 +114,7 @@ static unsigned long http_auth_methods = CURLAUTH_ANY;
> >  
> >  static struct curl_slist *pragma_header;
> >  static struct curl_slist *no_pragma_header;
> > +static struct curl_slist *extra_http_headers;
> >  
> >  static struct active_request_slot *active_queue_head;
> >  
> > @@ -323,6 +324,12 @@ static int http_options(const char *var, const char 
> > *value, void *cb)
> >  #endif
> >     }
> >  
> > +   if (!strcmp("http.extraheader", var)) {
> > +           extra_http_headers =
> > +                   curl_slist_append(extra_http_headers, value);
> > +           return 0;
> > +   }
> 
> I wondered if this would trigger for "http.*.extraheader", too. And it
> should, as that is all handled in the caller of http_options. Good.

Yes, I was surprised about that, too, but all the other http.* settings
are handled via the urlmatch mechanism (which rewrites the matching
http.<URL>.* settings).

> > @@ -678,8 +685,10 @@ void http_init(struct remote *remote, const char *url, 
> > int proactive_auth)
> >     if (remote)
> >             var_override(&http_proxy_authmethod, 
> > remote->http_proxy_authmethod);
> >  
> > -   pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
> > -   no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
> > +   pragma_header = curl_slist_append(http_get_default_headers(),
> > +           "Pragma: no-cache");
> > +   no_pragma_header = curl_slist_append(http_get_default_headers(),
> > +           "Pragma:");
> 
> This looked wrong to me at first, because we are appending to the
> default header list in each case. But the secret sauce is that calling
> http_get_default_headers() actually creates a _new_ list that is a copy
> of the default headers (and the caller can do what they will with it,
> and must free it).
> 
> I think that's really the only sane way to do it because of curl's
> interfaces. But maybe it is worth a comment either here, or along with
> http_get_default_headers(), or both.

I chose to rename it to http_copy_default_headers(); That should make it
easier to understand.

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

Reply via email to