Re: [PATCH v3] reflog: close leak of reflog expire entry
- From
Jeff King <peff@peff.net>
- Date
- Jul 22, 2025, 04:54 UTC
- Message-ID
- <20250722045456.GA824456@coredump.intra.peff.net>
- In-Reply-To
- <20250721-jk-fix-leak-reflog-expire-config-v3-1-c488b0586e80@gmail.com>
On Mon, Jul 21, 2025 at 04:39:37PM -0700, Jacob Keller wrote:
> Changes in v3: > - Remove the incorrect call in reflog_expiry_cleanup() > - Add a call in reflog_expire_condition() > - Link to v2: https://lore.kernel.org/r/20250709-jk-fix-leak-reflog-expire-config-v2-1-f9af934be8c1@gmail.com
This looks correct to me except...
Show 12 quoted lines
> diff --git a/builtin/gc.c b/builtin/gc.c > index 845876ff0286..37f543736599 100644 > --- a/builtin/gc.c > +++ b/builtin/gc.c > @@ -346,6 +346,7 @@ static int reflog_expire_condition(struct gc_config *cfg UNUSED) > count_reflog_entries, &data); > > reflog_expiry_cleanup(&data.policy); > + reflog_clear_expire_config(&data.policy); > return data.count >= data.limit; > } >
This needs to pass &data.policy.opts, no?
I think we might also want this test on top (or I'd be happy to see it squashed in). It shows off your fix when built with SANITIZE=leak, and also catches the bug that v2 of your patch had.
-Peff
-- >8 -- Subject: [PATCH] t1410: add test of gc.<pattern>.reflogExpire config
We have long supported the ability to set reflog expiration config for individual, going back to 3cb22b8efe (Per-ref reflog expiry configuration, 2008-06-15). But we have never had any tests.
Let's add a very basic one that checks that we apply the config correctly to a subset of refs (and not elsewhere). This also triggers the leaky code fixed by the previous commit.
Signed-off-by: Jeff King <peff@peff.net> --- t/t1410-reflog.sh | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh index 42b501f163..362e90d7d6 100755 --- a/t/t1410-reflog.sh +++ b/t/t1410-reflog.sh @@ -320,6 +320,34 @@ test_expect_success 'git reflog expire unknown reference' ' test_grep "error: reflog could not be found: ${SQ}does-not-exist${SQ}" stderr ' +test_expect_success 'expire with pattern config' ' + # Split refs/heads/ into two roots so we can apply config to each. Make + # two branches per root to verify that config is applied correctly + # multiple times. + git branch root1/branch1 && + git branch root1/branch2 && + git branch root2/branch1 && + git branch root2/branch2 && + + test_config "gc.reflogexpire" "never" && + test_config "gc.refs/heads/root2/*.reflogExpire" "now" && + git reflog expire \ + root1/branch1 root1/branch2 \ + root2/branch1 root2/branch2 && + + cat >expect <<-\EOF && + root1/branch1@{0} + root1/branch2@{0} + EOF + git log -g --branches="root*" --format=%gD >actual.raw && + # The sole reflog entry of each branch points to the same commit, so + # the order in which they are shown is nondeterministic. We just care + # about the what was expired (and what was not), so sort to get a known + # order. + sort <actual.raw >actual.sorted && + test_cmp expect actual.sorted +' + test_expect_success 'checkout should not delete log for packed ref' ' test $(git reflog main | wc -l) = 4 && git branch foo &&
-- 2.50.1.589.g6e88b11be3