From: Junio C Hamano <gitster@pobox•com>
To: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail•com>
Cc: git@vger•kernel.org, johannes.schindelin@gmx•de, peff@peff•net,
ps@pks•im, me@ttaylorr•com, johncai86@gmail•com,
newren@gmail•com, christian.couder@gmail•com,
kristofferhaugsbakk@fastmail•com, jonathantanmy@google•com,
karthik.188@gmail•com, Derrick Stolee <stolee@gmail•com>
Subject: Re: [PATCH 0/5] PATH WALK III: Add 'git backfill' command
Date: Mon, 09 Dec 2024 09:34:07 +0900 [thread overview]
Message-ID: <xmqqjzc9r8xs.fsf@gitster.g> (raw)
In-Reply-To: <xmqqy10qqwco.fsf@gitster.g> (Junio C. Hamano's message of "Sun, 08 Dec 2024 19:53:43 +0900")
Junio C Hamano <gitster@pobox•com> writes:
> It seems that the result of calling init_revisions() from backfill
> is leaked?
>
> https://github.com/git/git/actions/runs/12218430154/job/34083929479
>
> I did not dig further but the below is from my local leaksanitizer
> run.
The attached patch seems to plug the leaks observed by your backfill
tests. If you agree with the implementation of the change, you are
welcome to squash it in. I may be missing a better mechanism in the
path-walk API that I could use to plug the leaks, in which case, of
course a fix using such a better mechanism is very much welcomed.
A few things I noticed about path-walk API during my poking are:
- struct path_walk_info has PATH_WALK_INFO_INIT() macro, but not a
matching path_walk_info_init() helper function to help initialize
any heap-allocated instances. In general, anything that requires
_INIT() macro by definition wants to be initialized with more
than nul-initialized (otherwise, it would be fine to leave it
uninitialized in the BSS, clear with = {0} initialization on the
stack), so lack of matching path_walk_info_init() raises an
eyebrow.
- struct path_walk_info seems to lack a destructor. In general,
anything complex enough to require initializer should have one.
- lack of destructor for path_walk_info requires users to release
resources held by "struct pattern_list" instance at pwi.pl; if we
had a destructor, we wouldn't have 2 of the three leaks we had to
fix here.
Thanks.
diff --git c/builtin/backfill.c w/builtin/backfill.c
index 225764f17e..1cf2df9303 100644
--- c/builtin/backfill.c
+++ w/builtin/backfill.c
@@ -92,8 +92,11 @@ static int do_backfill(struct backfill_context *ctx)
if (ctx->sparse) {
CALLOC_ARRAY(info.pl, 1);
- if (get_sparse_checkout_patterns(info.pl))
+ if (get_sparse_checkout_patterns(info.pl)) {
+ clear_pattern_list(info.pl);
+ free(info.pl);
return error(_("problem loading sparse-checkout"));
+ }
}
repo_init_revisions(ctx->repo, &revs, "");
@@ -113,6 +116,11 @@ static int do_backfill(struct backfill_context *ctx)
download_batch(ctx);
clear_backfill_context(ctx);
+ release_revisions(&revs);
+ if (info.pl) {
+ clear_pattern_list(info.pl);
+ free(info.pl);
+ }
return ret;
}
next prev parent reply other threads:[~2024-12-09 0:34 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-06 20:07 [PATCH 0/5] PATH WALK III: Add 'git backfill' command Derrick Stolee via GitGitGadget
2024-12-06 20:07 ` [PATCH 1/5] backfill: add builtin boilerplate Derrick Stolee via GitGitGadget
2025-01-16 10:11 ` Patrick Steinhardt
2025-01-16 17:52 ` Junio C Hamano
2025-02-03 14:38 ` Derrick Stolee
2024-12-06 20:07 ` [PATCH 2/5] backfill: basic functionality and tests Derrick Stolee via GitGitGadget
2024-12-16 8:01 ` Patrick Steinhardt
2024-12-18 15:03 ` Derrick Stolee
2024-12-06 20:07 ` [PATCH 3/5] backfill: add --batch-size=<n> option Derrick Stolee via GitGitGadget
2024-12-16 8:01 ` Patrick Steinhardt
2024-12-18 15:09 ` Derrick Stolee
2025-01-19 17:57 ` Jean-Noël AVILA
2024-12-06 20:07 ` [PATCH 4/5] backfill: add --sparse option Derrick Stolee via GitGitGadget
2024-12-16 8:01 ` Patrick Steinhardt
2024-12-06 20:07 ` [PATCH 5/5] backfill: assume --sparse when sparse-checkout is enabled Derrick Stolee via GitGitGadget
2024-12-08 10:53 ` [PATCH 0/5] PATH WALK III: Add 'git backfill' command Junio C Hamano
2024-12-09 0:34 ` Junio C Hamano [this message]
2024-12-20 16:29 ` [PATCH v2 " Derrick Stolee via GitGitGadget
2024-12-20 16:29 ` [PATCH v2 1/5] backfill: add builtin boilerplate Derrick Stolee via GitGitGadget
2024-12-20 16:29 ` [PATCH v2 2/5] backfill: basic functionality and tests Derrick Stolee via GitGitGadget
2025-01-16 10:01 ` Patrick Steinhardt
2025-02-03 14:44 ` Derrick Stolee
2024-12-20 16:29 ` [PATCH v2 3/5] backfill: add --min-batch-size=<n> option Derrick Stolee via GitGitGadget
2025-01-16 10:01 ` Patrick Steinhardt
2024-12-20 16:29 ` [PATCH v2 4/5] backfill: add --sparse option Derrick Stolee via GitGitGadget
2025-01-16 10:01 ` Patrick Steinhardt
2025-02-03 15:11 ` Derrick Stolee
2024-12-20 16:29 ` [PATCH v2 5/5] backfill: assume --sparse when sparse-checkout is enabled Derrick Stolee via GitGitGadget
2025-01-16 10:00 ` [PATCH v2 0/5] PATH WALK III: Add 'git backfill' command Patrick Steinhardt
2025-01-17 22:37 ` Junio C Hamano
2025-02-03 17:11 ` [PATCH v3 " Derrick Stolee via GitGitGadget
2025-02-03 17:11 ` [PATCH v3 1/5] backfill: add builtin boilerplate Derrick Stolee via GitGitGadget
2025-02-03 17:11 ` [PATCH v3 2/5] backfill: basic functionality and tests Derrick Stolee via GitGitGadget
2025-02-03 17:11 ` [PATCH v3 3/5] backfill: add --min-batch-size=<n> option Derrick Stolee via GitGitGadget
2025-02-03 17:11 ` [PATCH v3 4/5] backfill: add --sparse option Derrick Stolee via GitGitGadget
2025-02-03 17:11 ` [PATCH v3 5/5] backfill: assume --sparse when sparse-checkout is enabled Derrick Stolee via GitGitGadget
2025-02-04 0:18 ` [PATCH v3 0/5] PATH WALK III: Add 'git backfill' command Junio C Hamano
2025-02-05 7:15 ` Patrick Steinhardt
2025-02-05 17:07 ` Junio C Hamano
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=xmqqjzc9r8xs.fsf@gitster.g \
--to=gitster@pobox$(echo .)com \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=johannes.schindelin@gmx$(echo .)de \
--cc=johncai86@gmail$(echo .)com \
--cc=jonathantanmy@google$(echo .)com \
--cc=karthik.188@gmail$(echo .)com \
--cc=kristofferhaugsbakk@fastmail$(echo .)com \
--cc=me@ttaylorr$(echo .)com \
--cc=newren@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=ps@pks$(echo .)im \
--cc=stolee@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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