{"thread":{"id":"64651","subject":"[PATCH] refs: dereference the value of the required pointer","startedAt":"2025-12-18T16:10:52Z","lastAt":"2025-12-25T21:23:23Z","messageCount":4,"participants":["AZero13 via GitGitGadget","Junio C Hamano","Patrick Steinhardt","Karthik Nayak"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"532475","messageId":"pull.2130.git.git.1766074249443.gitgitgadget@gmail.com","threadId":"64651","inReplyTo":null,"subject":"[PATCH] refs: dereference the value of the required pointer","fromName":"AZero13 via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-12-18T16:10:49Z","receivedAt":"2025-12-18T16:10:52Z","isPatch":true,"sender":{"key":"name:AZero13","avatar":null},"body":"From: Greg Funni \n\nCurrently, this always prints yes because required is non-null.\n\nThis is the wrong behavior. The boolean must be\ndereferenced.\n\nSigned-off-by: Greg Funni \n---\n refs: dereference the value of the required pointer\n \n Currently, this always prints yes because required is non-null.\n \n This is the wrong behavior. The boolean must be dereferenced.\n \n Signed-off-by: Greg Funni gfunni234@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2130%2FAZero13%2Fref-cache-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2130/AZero13/ref-cache-v1\nPull-Request: https://github.com/git/git/pull/2130\n\n refs/debug.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 3e31228c9a..639db0f26e 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -139,7 +139,7 @@ static int debug_optimize_required(struct ref_store *ref_store,\n \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n \tint res = drefs->refs->be->optimize_required(drefs->refs, opts, required);\n \ttrace_printf_key(&trace_refs, \"optimize_required: %s, res: %d\\n\",\n-\t\t\t required ? \"yes\" : \"no\", res);\n+\t\t\t *required ? \"yes\" : \"no\", res);\n \treturn res;\n }\n \n\nbase-commit: c4a0c8845e2426375ad257b6c221a3a7d92ecfda\n-- \ngitgitgadget\n"},{"id":"532514","messageId":"xmqqzf7fw2hd.fsf@gitster.g","threadId":"64651","inReplyTo":"pull.2130.git.git.1766074249443.gitgitgadget@gmail.com","subject":"Re: [PATCH] refs: dereference the value of the required pointer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-19T03:54:22Z","receivedAt":"2025-12-19T03:54:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"AZero13 via GitGitGadget\" writes:\n\n> From: Greg Funni \n>\n> Currently, this always prints yes because required is non-null.\n>\n> This is the wrong behavior. The boolean must be\n> dereferenced.\n\nThe line is blamed to f6c5ca38 (refs: add a `optimize_required`\nfield to `struct ref_storage_be`, 2025-11-08); the author CC'ed for\nan Ack.\n\nThanks.\n\n>\n> Signed-off-by: Greg Funni \n> ---\n> refs: dereference the value of the required pointer\n> \n> Currently, this always prints yes because required is non-null.\n> \n> This is the wrong behavior. The boolean must be dereferenced.\n> \n> Signed-off-by: Greg Funni gfunni234@gmail.com\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2130%2FAZero13%2Fref-cache-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2130/AZero13/ref-cache-v1\n> Pull-Request: https://github.com/git/git/pull/2130\n>\n> refs/debug.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/refs/debug.c b/refs/debug.c\n> index 3e31228c9a..639db0f26e 100644\n> --- a/refs/debug.c\n> +++ b/refs/debug.c\n> @@ -139,7 +139,7 @@ static int debug_optimize_required(struct ref_store *ref_store,\n> \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n> \tint res = drefs->refs->be->optimize_required(drefs->refs, opts, required);\n> \ttrace_printf_key(&trace_refs, \"optimize_required: %s, res: %d\\n\",\n> -\t\t\t required ? \"yes\" : \"no\", res);\n> +\t\t\t *required ? \"yes\" : \"no\", res);\n> \treturn res;\n> }\n> \n>\n> base-commit: c4a0c8845e2426375ad257b6c221a3a7d92ecfda\n"},{"id":"532518","messageId":"aUTwmSNfaoVzEIpD@pks.im","threadId":"64651","inReplyTo":"pull.2130.git.git.1766074249443.gitgitgadget@gmail.com","subject":"Re: [PATCH] refs: dereference the value of the required pointer","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-19T06:28:41Z","receivedAt":"2025-12-19T06:28:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Dec 18, 2025 at 04:10:49PM +0000, AZero13 via GitGitGadget wrote:\n> diff --git a/refs/debug.c b/refs/debug.c\n> index 3e31228c9a..639db0f26e 100644\n> --- a/refs/debug.c\n> +++ b/refs/debug.c\n> @@ -139,7 +139,7 @@ static int debug_optimize_required(struct ref_store *ref_store,\n> \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n> \tint res = drefs->refs->be->optimize_required(drefs->refs, opts, required);\n> \ttrace_printf_key(&trace_refs, \"optimize_required: %s, res: %d\\n\",\n> -\t\t\t required ? \"yes\" : \"no\", res);\n> +\t\t\t *required ? \"yes\" : \"no\", res);\n> \treturn res;\n> }\n\nMakes sense. One question is whether `required` will always be non-NULL\nso that we can unconditionally dereference the pointer like this. But\nfrom going through the implementations I can see that the pointer\nalready does get dereferenced unconditionally, so this fix is safe.\n\nThanks!\n\nPatrick\n"},{"id":"532741","messageId":"CAOLa=ZQwrdXOocxB1A5TyGYBecQYcM2r2p8ZUZfBiav04cuSGw@mail.gmail.com","threadId":"64651","inReplyTo":"xmqqzf7fw2hd.fsf@gitster.g","subject":"Re: [PATCH] refs: dereference the value of the required pointer","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-12-25T21:23:20Z","receivedAt":"2025-12-25T21:23:23Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> \"AZero13 via GitGitGadget\" writes:\n>\n>> From: Greg Funni \n>>\n>> Currently, this always prints yes because required is non-null.\n>>\n>> This is the wrong behavior. The boolean must be\n>> dereferenced.\n>\n> The line is blamed to f6c5ca38 (refs: add a `optimize_required`\n> field to `struct ref_storage_be`, 2025-11-08); the author CC'ed for\n> an Ack.\n>\n> Thanks.\n>\n\nMy responses are a bit slow due to being on holiday.\n\nThe patch looks good to me, the fix makes sense. Thanks both!\n"}]}