Re: [PATCH v2 1/1] remote-curl: unbreak http.extraHeader with custom allocators

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 1981 bytes --]

Hi Junio,

On Wed, 6 Nov 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > 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.
>
> s/allocator/de&/; perhaps, even though it is clear enough from the
> context, so it is probably OK as is.
>
> > A naïve attempt at fixing this would move the call to
> > `curl_global_init()` _before_ the config is parsed (i.e. before that
> > call to `slist_append()`).
> >
> > However, that does work, as we _also_ parse the config setting
>
> s/does work/does not work/; presumably?

Indeed. Could I ask you to fix up locally, or do you want me to send a
new revision of the patch?

Ciao,
Dscho

>
> > `http.sslbackend` and if found, call `curl_global_sslset()` which *must*
> > be called before `curl_global_init()`, for details see:
> > https://curl.haxx.se/libcurl/c/curl_global_sslset.html
> >
> > So let's instead make the config parsing entirely independent from
> > cURL's data structures. Incidentally, this deletes two more lines than
> > it introduces, which is nice.
>
> Yeah, string_list_clear() is more concise than curl_slist_free_all(),
> and we have already been copying one list to another anyway, so we
> lucked out ;-)
>
> The patch looked good to me, too.
>