Re: [PATCH v3 03/11] http: store all request headers on active_request_slot

[Glen Choo <chooglen@google.com>]

"Matthew John Cheetham via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> Once a list of headers has been set on the curl handle, it is not
> possible to recover that `struct curl_slist` instance to add or modify
> headers.
>
> In future commits we will want to modify the set of request headers in
> response to an authentication challenge/401 response from the server,
> with information provided by a credential helper.
>
> There are a number of different places where curl is used for an HTTP
> request, and they do not have a common handling of request headers.
> However, given that they all do call the `start_active_slot()` function,
> either directly or indirectly via `run_slot()` or `run_one_slot()`, we
> use this as the point to set the `CURLOPT_HTTPHEADER` option just
> before the request is made.
>
> We collect all request headers in a `struct curl_slist` on the
> `struct active_request_slot` that is obtained from a call to
> `get_active_slot(int)`. This function now takes a single argument to
> define if the initial set of headers on the slot should include the
> "Pragma: no-cache" header, along with all extra headers specified via
> `http.extraHeader` config values.

I admit that I'm not that familiar with the http subsystem, so I'll
focus on the style.

If I'm reading this patch correctly, there are two related, but distinct
changes:

- store and modify the headers on the slot
- change how headers are initialized and remove now-unncessary libcurl
  calls that set headers

Both are simple, but given the number of LoCs changed, I found it quite
difficult to track which LoCs were part of which work. Could this be
broken up into two patches instead, i.e.:

- store headers on the slot without changing how they are initialized
- add extra header initialization logic to get_active_slot() and remove
  the unnecessary libcurl calls