Re: [PATCH v2 3/4] apply: remove the_repository global variable
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 30, 2024, 20:06 UTC
- Message-ID
- <xmqqy13852jk.fsf@gitster.g>
- In-Reply-To
- <d64955a2e277da138146020f6a0cf96f4636a162.1727718031.git.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 34 quoted lines
> From: John Cai <johncai86@gmail.com>
>
> Remove the_repository global variable in favor of the repository
> argument that gets passed in through the builtin function.
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> builtin/apply.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 84f1863d3ac..d0bafbec7e4 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
> #include "builtin.h"
> #include "gettext.h"
> #include "hash.h"
> @@ -12,14 +11,14 @@ static const char * const apply_usage[] = {
> int cmd_apply(int argc,
> const char **argv,
> const char *prefix,
> - struct repository *repo UNUSED)
> + struct repository *repo)
> {
> int force_apply = 0;
> int options = 0;
> int ret;
> struct apply_state state;
>
> - if (init_apply_state(&state, the_repository, prefix))
> + if (init_apply_state(&state, repo, prefix))
> exit(128);Is this one, and ...
Show 10 quoted lines
> > /* > @@ -28,8 +27,8 @@ int cmd_apply(int argc, > * is worth the effort. > * cf. https://lore.kernel.org/git/xmqqcypfcmn4.fsf@gitster.g/ > */ > - if (!the_hash_algo) > - repo_set_hash_algo(the_repository, GIT_HASH_SHA1); > + if (!repo->hash_algo) > + repo_set_hash_algo(repo, GIT_HASH_SHA1);
... is this use of "repo" still valid? We now pass NULL, not the_repository, when a command with SETUP_GENTLY is asked to run outside a repository, no? Shouldn't it detecting the case, and passing the pointer to a fallback object (perhaps the_repository) instead of repo?
I _think_ state->repo->index is accessed unconditionally only to figure out whitespace attributes, even outside a repository (thanks to the_repository standing in), with the expectation that the index is empty (because we do not read any) and we find no customization, when "git apply" is used as a better GNU patch to work outside any repository. Maybe I am not looking hard enough, but I fail to see how the code makes sure that repo being NULL outside a repository does not lead to a dereferencing of a NULL pointer.
Thanks.