git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH 02/13] sparse-checkout: pass string literals directly to add_pattern()
Date: Fri, 31 May 2024 09:14:49 -0700	[thread overview]
Message-ID: <xmqqr0digdqe.fsf@gitster.g> (raw)
In-Reply-To: <20240531112613.GB428814@coredump.intra.peff.net> (Jeff King's message of "Fri, 31 May 2024 07:26:13 -0400")

Jeff King <peff@peff.net> writes:

> The add_pattern() function takes a pattern string, but neither makes a
> copy of it nor takes ownership of the memory. So it is the caller's
> responsibility to make sure the string hangs around as long as the
> pattern_list which references it.

Wow.  That's an extremely bad API.

I suspect that it long time ago did not aggressively "pre-parse" the
given pattern string and kept the original copy in the struct (i.e.,
taking ownership), and these copies of the literal strings were made
at the calling sites with the expectation that the API takes
ownership of them, as most of the strings fed to add_pattern() are
what we read from the environment into heap.

> In the long run, we will also want to clean up this (undocumented!)
> memory lifetime requirement of add_pattern(). But that can come in a
> later patch; passing the string literals directly will be the right
> thing either way.

OK.

  reply	other threads:[~2024-05-31 16:14 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-31 11:24 [PATCH 0/13] leak fixes for sparse-checkout code Jeff King
2024-05-31 11:25 ` [PATCH 01/13] sparse-checkout: free string list in write_cone_to_file() Jeff King
2024-05-31 11:26 ` [PATCH 02/13] sparse-checkout: pass string literals directly to add_pattern() Jeff King
2024-05-31 16:14   ` Junio C Hamano [this message]
2024-06-04  9:43     ` Jeff King
2024-05-31 11:26 ` [PATCH 03/13] dir.c: free strings in sparse cone pattern hashmaps Jeff King
2024-05-31 11:27 ` [PATCH 04/13] sparse-checkout: clear patterns when init() sees existing sparse file Jeff King
2024-05-31 11:28 ` [PATCH 05/13] dir.c: free removed sparse-pattern hashmap entries Jeff King
2024-05-31 11:31 ` [PATCH 06/13] dir.c: always copy input to add_pattern() Jeff King
2024-05-31 22:28   ` Junio C Hamano
2024-05-31 11:31 ` [PATCH 07/13] sparse-checkout: reuse --stdin buffer when reading patterns Jeff King
2024-05-31 11:34 ` [PATCH 08/13] sparse-checkout: always free "line" strbuf after reading input Jeff King
2024-05-31 11:35 ` [PATCH 09/13] sparse-checkout: refactor temporary sparse_checkout_patterns Jeff King
2024-06-04  7:42   ` Patrick Steinhardt
2024-06-04  9:53     ` Jeff King
2024-05-31 11:35 ` [PATCH 10/13] sparse-checkout: free sparse_filename after use Jeff King
2024-06-04  7:42   ` Patrick Steinhardt
2024-06-04 10:01     ` Jeff King
2024-06-04 12:13       ` Patrick Steinhardt
2024-05-31 11:36 ` [PATCH 11/13] sparse-checkout: free pattern list in sparse_checkout_list() Jeff King
2024-05-31 11:36 ` [PATCH 12/13] sparse-checkout: free string list after displaying Jeff King
2024-05-31 11:38 ` [PATCH 13/13] sparse-checkout: free duplicate hashmap entries Jeff King
2024-06-04  7:43   ` Patrick Steinhardt
2024-05-31 11:49 ` [PATCH 0/13] leak fixes for sparse-checkout code Jeff King
2024-05-31 15:01   ` Junio C Hamano
2024-06-04 10:08 ` [PATCH v2 " Jeff King
2024-06-04 10:13   ` [PATCH v2 01/13] sparse-checkout: free string list in write_cone_to_file() Jeff King
2024-06-04 10:13   ` [PATCH v2 02/13] sparse-checkout: pass string literals directly to add_pattern() Jeff King
2024-06-04 10:13   ` [PATCH v2 03/13] dir.c: free strings in sparse cone pattern hashmaps Jeff King
2024-06-04 10:13   ` [PATCH v2 04/13] sparse-checkout: clear patterns when init() sees existing sparse file Jeff King
2024-06-04 10:13   ` [PATCH v2 05/13] dir.c: free removed sparse-pattern hashmap entries Jeff King
2024-06-04 10:13   ` [PATCH v2 06/13] dir.c: always copy input to add_pattern() Jeff King
2024-06-05  8:53     ` René Scharfe
2024-06-05  9:18       ` Jeff King
2024-06-05 16:52         ` Junio C Hamano
2024-06-04 10:13   ` [PATCH v2 07/13] sparse-checkout: reuse --stdin buffer when reading patterns Jeff King
2024-06-04 10:13   ` [PATCH v2 08/13] sparse-checkout: always free "line" strbuf after reading input Jeff King
2024-06-04 10:13   ` [PATCH v2 09/13] sparse-checkout: refactor temporary sparse_checkout_patterns Jeff King
2024-06-04 10:13   ` [PATCH v2 10/13] sparse-checkout: free sparse_filename after use Jeff King
2024-06-04 10:13   ` [PATCH v2 11/13] sparse-checkout: free pattern list in sparse_checkout_list() Jeff King
2024-06-04 10:13   ` [PATCH v2 12/13] sparse-checkout: free string list after displaying Jeff King
2024-06-04 10:13   ` [PATCH v2 13/13] sparse-checkout: free duplicate hashmap entries Jeff King
2024-06-04 12:15   ` [PATCH v2 0/13] leak fixes for sparse-checkout code Patrick Steinhardt

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

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

  git send-email \
    --in-reply-to=xmqqr0digdqe.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /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.
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 NNTP newsgroup(s).