From: Patrick Steinhardt <ps@pks.im>
To: RuanXinyu via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, RuanXinyu <1096421257@qq.com>,
RuanXinyu <r200981113@gmail.com>
Subject: Re: [PATCH] refs: add missing remove_on_disk implementation for debug backend
Date: Fri, 24 Oct 2025 11:11:27 +0200 [thread overview]
Message-ID: <aPtCvwvNUtFXqrpv@pks.im> (raw)
In-Reply-To: <pull.2082.git.git.1761295094982.gitgitgadget@gmail.com>
On Fri, Oct 24, 2025 at 08:38:14AM +0000, RuanXinyu via GitGitGadget wrote:
> From: RuanXinyu <r200981113@gmail.com>
>
> The debug ref backend (refs_be_debug) was missing the remove_on_disk
> function pointer, which caused a segmentation fault when running
> 'GIT_TRACE_REFS=1 git refs migrate --ref-format=reftable' commands.
Heh, funny, just as I said nobody uses this infra you show up :) Good
way to prove me wrong, thanks!
> Signed-off-by: Xinyu Ruan <r200981113@gmail.com>
Tiny nit: typically, the author and DCO should match. But the autor is
"RuanXinyu" whereas the DCO says "Xinyu Ruan". I don't really think that
this is something that warrants a new version, but I wanted to point
this out anyway so that you can fix this going forward.
> diff --git a/refs/debug.c b/refs/debug.c
> index da300efaf3..dd49080836 100644
> --- a/refs/debug.c
> +++ b/refs/debug.c
> @@ -48,6 +48,14 @@ static int debug_create_on_disk(struct ref_store *refs, int flags, struct strbuf
> return res;
> }
>
> +static int debug_remove_on_disk(struct ref_store *refs, struct strbuf *err)
> +{
> + struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
> + int res = drefs->refs->be->remove_on_disk(drefs->refs, err);
> + trace_printf_key(&trace_refs, "remove_on_disk: %d\n", res);
> + return res;
> +}
> +
> static int debug_transaction_prepare(struct ref_store *refs,
> struct ref_transaction *transaction,
> struct strbuf *err)
> @@ -432,6 +440,7 @@ struct ref_storage_be refs_be_debug = {
> .init = NULL,
> .release = debug_release,
> .create_on_disk = debug_create_on_disk,
> + .remove_on_disk = debug_remove_on_disk,
>
> /*
> * None of these should be NULL. If the "files" backend (in
Yup, the implementation looks obviously correct to me. Thanks for fixing
this bug!
Patrick
next prev parent reply other threads:[~2025-10-24 9:11 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-24 8:38 [PATCH] refs: add missing remove_on_disk implementation for debug backend RuanXinyu via GitGitGadget
2025-10-24 9:11 ` Patrick Steinhardt [this message]
2025-10-24 15:13 ` Junio C Hamano
2025-10-27 4:01 ` 阮新宇
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=aPtCvwvNUtFXqrpv@pks.im \
--to=ps@pks.im \
--cc=1096421257@qq.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=r200981113@gmail.com \
/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).