git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Nov 6, 2019, 19:34 UTC
Message-ID
<nycvar.QRO.7.76.6.1911062031480.46@tvgsbejvaqbjf.bet>
In-Reply-To
<xmqq7e4dtd9n.fsf@gitster-ct.c.googlers.com>
Hi Junio,
On Wed, 6 Nov 2019, Junio C Hamano wrote:
Show 28 quoted lines
> "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

Show 15 quoted lines
>
> > `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.
>
Previous: Junio C HamanoNext: Junio C Hamano
Message 13 of 17 in “remote-curl: unbreak http.extraHeader with custom allocators”
  1. 0/1 remote-curl: unbreak http.extraHeader with custom allocatorsJohannes Schindelin via GitGitGadget, Nov 5, 2019
  2. 1/1 remote-curl: unbreak http.extraHeader with custom allocatorsJohannes Schindelin via GitGitGadget, Nov 5, 2019
  3. Jeff KingNov 6, 2019
  4. Johannes SchindelinNov 6, 2019
  5. Junio C HamanoNov 6, 2019
  6. Johannes SchindelinNov 6, 2019
  7. Junio C HamanoNov 8, 2019
  8. Johannes SchindelinNov 8, 2019
  9. 0/1 remote-curl: unbreak http.extraHeader with custom allocatorsJohannes Schindelin via GitGitGadget, Nov 6, 2019
  10. 1/1 remote-curl: unbreak http.extraHeader with custom allocatorsJohannes Schindelin via GitGitGadget, Nov 6, 2019
  11. Jeff KingNov 6, 2019
  12. Junio C HamanoNov 6, 2019
  13. Johannes SchindelinNov 6, 2019
  14. Junio C HamanoNov 7, 2019
  15. Johannes SchindelinNov 7, 2019
  16. Junio C HamanoNov 8, 2019
  17. Junio C HamanoNov 6, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.