# [PATCH] refs: dereference the value of the required pointer 4 messages from 2025-12-18 to 2025-12-25. Participants: AZero13 via GitGitGadget, Junio C Hamano, Patrick Steinhardt, Karthik Nayak. Thread: https://gitlist.dev/t/64651 ## AZero13 via GitGitGadget, 2025-12-18 16:10 Subject: [PATCH] refs: dereference the value of the required pointer Message-ID: URL: https://gitlist.dev/e/pull.2130.git.git.1766074249443.gitgitgadget%40gmail.com ``` From: Greg Funni Currently, this always prints yes because required is non-null. This is the wrong behavior. The boolean must be dereferenced. Signed-off-by: Greg Funni --- refs: dereference the value of the required pointer Currently, this always prints yes because required is non-null. This is the wrong behavior. The boolean must be dereferenced. Signed-off-by: Greg Funni gfunni234@gmail.com Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2130%2FAZero13%2Fref-cache-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2130/AZero13/ref-cache-v1 Pull-Request: https://github.com/git/git/pull/2130 refs/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs/debug.c b/refs/debug.c index 3e31228c9a..639db0f26e 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -139,7 +139,7 @@ static int debug_optimize_required(struct ref_store *ref_store, struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; int res = drefs->refs->be->optimize_required(drefs->refs, opts, required); trace_printf_key(&trace_refs, "optimize_required: %s, res: %d\n", - required ? "yes" : "no", res); + *required ? "yes" : "no", res); return res; } base-commit: c4a0c8845e2426375ad257b6c221a3a7d92ecfda -- gitgitgadget ``` ## Junio C Hamano, 2025-12-19 03:54 Subject: Re: [PATCH] refs: dereference the value of the required pointer Message-ID: URL: https://gitlist.dev/e/xmqqzf7fw2hd.fsf%40gitster.g In-Reply-To: ``` "AZero13 via GitGitGadget" writes: > From: Greg Funni > > Currently, this always prints yes because required is non-null. > > This is the wrong behavior. The boolean must be > dereferenced. The line is blamed to f6c5ca38 (refs: add a `optimize_required` field to `struct ref_storage_be`, 2025-11-08); the author CC'ed for an Ack. Thanks. > > Signed-off-by: Greg Funni > --- > refs: dereference the value of the required pointer > > Currently, this always prints yes because required is non-null. > > This is the wrong behavior. The boolean must be dereferenced. > > Signed-off-by: Greg Funni gfunni234@gmail.com > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2130%2FAZero13%2Fref-cache-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2130/AZero13/ref-cache-v1 > Pull-Request: https://github.com/git/git/pull/2130 > > refs/debug.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/refs/debug.c b/refs/debug.c > index 3e31228c9a..639db0f26e 100644 > --- a/refs/debug.c > +++ b/refs/debug.c > @@ -139,7 +139,7 @@ static int debug_optimize_required(struct ref_store *ref_store, > struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; > int res = drefs->refs->be->optimize_required(drefs->refs, opts, required); > trace_printf_key(&trace_refs, "optimize_required: %s, res: %d\n", > - required ? "yes" : "no", res); > + *required ? "yes" : "no", res); > return res; > } > > > base-commit: c4a0c8845e2426375ad257b6c221a3a7d92ecfda ``` ## Patrick Steinhardt, 2025-12-19 06:28 Subject: Re: [PATCH] refs: dereference the value of the required pointer Message-ID: URL: https://gitlist.dev/e/aUTwmSNfaoVzEIpD%40pks.im In-Reply-To: ``` On Thu, Dec 18, 2025 at 04:10:49PM +0000, AZero13 via GitGitGadget wrote: > diff --git a/refs/debug.c b/refs/debug.c > index 3e31228c9a..639db0f26e 100644 > --- a/refs/debug.c > +++ b/refs/debug.c > @@ -139,7 +139,7 @@ static int debug_optimize_required(struct ref_store *ref_store, > struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; > int res = drefs->refs->be->optimize_required(drefs->refs, opts, required); > trace_printf_key(&trace_refs, "optimize_required: %s, res: %d\n", > - required ? "yes" : "no", res); > + *required ? "yes" : "no", res); > return res; > } Makes sense. One question is whether `required` will always be non-NULL so that we can unconditionally dereference the pointer like this. But from going through the implementations I can see that the pointer already does get dereferenced unconditionally, so this fix is safe. Thanks! Patrick ``` ## Karthik Nayak, 2025-12-25 21:23 Subject: Re: [PATCH] refs: dereference the value of the required pointer Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZQwrdXOocxB1A5TyGYBecQYcM2r2p8ZUZfBiav04cuSGw%40mail.gmail.com In-Reply-To: ``` Junio C Hamano writes: > "AZero13 via GitGitGadget" writes: > >> From: Greg Funni >> >> Currently, this always prints yes because required is non-null. >> >> This is the wrong behavior. The boolean must be >> dereferenced. > > The line is blamed to f6c5ca38 (refs: add a `optimize_required` > field to `struct ref_storage_be`, 2025-11-08); the author CC'ed for > an Ack. > > Thanks. > My responses are a bit slow due to being on holiday. The patch looks good to me, the fix makes sense. Thanks both! ```