Hi Peff, On Tue, 5 Nov 2019, Jeff King wrote: > On Tue, Nov 05, 2019 at 09:59:18PM +0000, Johannes Schindelin via GitGitGadget wrote: > > > From: Johannes Schindelin <johannes.schindelin@xxxxxx> > > > > In 93b980e58f5 (http: use xmalloc with cURL, 2019-08-15), we started to > > ask cURL to use `xmalloc()`, and if compiled with nedmalloc, that means > > implicitly a different allocator than the system one. > > > > Which means that all of cURL's allocations and releases now _need_ to > > use that allocator. > > > > However, the `http_options()` function used `slist_append()` to add any > > configured extra HTTP header(s) _before_ asking cURL to use `xmalloc()`, > > and `http_cleanup()` would release them _afterwards_, i.e. in the > > presence of custom allocators, cURL would attempt to use the wrong > > allocator to release the memory. > > > > Let's fix this by moving the initialization _before_ the > > `http_options()` function is called. > > Nicely explained. > > Another option would be to separate our config mechanism from curl > entirely by putting the list of headers into a string_list, and then > transforming it later into a curl_slist. I don't think that really buys > us much, though. Alas, it _does_ buy us a lot, as I *just* found out (can you imagine how glad I am not to have rushed out another Git for Windows release?). It buys us one less bug: my patch introduces the bug where `http.sslbackend` can no longer be used, because `curl_global_sslset()` needs to be called _before_ `curl_global_init()`, but my patch breaks that because we _need_ to parse the config before we can ask cURL for a specific backend. > This is all inside http.c, so it's fairly contained. It's not like > other random parts of Git are using curl's slist before calling > http_init(). Indeed. We cannot use cURL's slist anywhere outside of the cURL-dependent code because we want to keep `NO_CURL=Yep` working. > I did briefly grep around for other slist users, but they're all what > you'd expect: code in http-push.c and remote-curl.c creating header > lists while working with an active http request (so well after > http_init() has been called). Indeed, I came to the same conclusion that Carlo's patch only broke support for `http.extraheaders` (and only with custom allocators), nothing else. I will change the patch to avoid using `slist` early and send another iteration. Thanks, Dscho > > --- > > http.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > The patch itself looks good. > > -Peff >
