git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Carlo Arenas <carenas@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, l.s.r@web.de
Subject: Re: [RFC PATCH] http: use xmalloc with cURL
Date: Sun, 11 Aug 2019 12:08:53 -0700	[thread overview]
Message-ID: <CAPUEspg62pRNH6=_VvWDxQ4YujHUJAoTTampc0L4t68QMj30xg@mail.gmail.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1908111317540.46@tvgsbejvaqbjf.bet>

On Sun, Aug 11, 2019 at 4:20 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Sat, 10 Aug 2019, Carlo Marcelo Arenas Belón wrote:
> > cURL is very strict about its allocator being thread safe and so that might
> > be an issue to look for.
>
> Looks good to me.

it is not ready yet for using though, at minimum I have to fix the #ifdef
so there is no risky of breaking someone's build because they happen
to still be using and old enough cURL (ex: 7.10)

there is also the risk that xmalloc might not be sufficiently thread
safe (ex: when it triggers unuse_one_window() through the use of a
try_to_free_routine in packfile.c but we could mitigate the risk for
now by doing it only first #ifdef NED (I promise to take it out before
it even gets to next, assuming Junio would trust me enough after the
last blow up to even get it into pu)

alternatively, getting some feedback from people that know that code
better like Shawn, since I have to admit I haven't read it enough to
be fully confident it is safe (although I believe it was enough, or I
wouldn't had sent it for feedback otherwise)

> Please note that this did not cause problems in Git for Windows.
> Probably because we leave it to cURL to deallocate whatever it
> allocated.

unlike PCRE2 there is no layering violation to trigger

> Still, it is a good idea to ask libcurl to use the same allocator as Git.

and it should have a positive performance impact, which should be nice
to look for.

Carlo

  reply	other threads:[~2019-08-11 19:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-10 22:02 [RFC PATCH] http: use xmalloc with cURL Carlo Marcelo Arenas Belón
2019-08-10 22:17 ` Daniel Stenberg
2019-08-10 22:41   ` Carlo Arenas
2019-08-11 11:20 ` Johannes Schindelin
2019-08-11 19:08   ` Carlo Arenas [this message]
2019-08-12 19:23     ` Johannes Schindelin
2019-08-12 19:55     ` Jeff King
2019-08-12 20:04       ` Junio C Hamano
2019-08-12 20:50         ` Jeff King
2019-08-13 20:00         ` Johannes Schindelin
2019-08-14 16:01           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPUEspg62pRNH6=_VvWDxQ4YujHUJAoTTampc0L4t68QMj30xg@mail.gmail.com' \
    --to=carenas@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).