[PATCH] refs: dereference the value of the required pointer

[PATCH] refs: dereference the value of the required pointer

[AZero13 via GitGitGadget]

From: Greg Funni <gfunni234@gmail.com>

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>
---
    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

Re: [PATCH] refs: dereference the value of the required pointer

[Junio C Hamano]

"AZero13 via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Greg Funni <gfunni234@gmail.com>
>
> 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 <gfunni234@gmail.com>
> ---
>     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

Re: [PATCH] refs: dereference the value of the required pointer

[Patrick Steinhardt]

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

Re: [PATCH] refs: dereference the value of the required pointer

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> "AZero13 via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Greg Funni <gfunni234@gmail.com>
>>
>> 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!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]