Re: [PATCH] git: avoid segfault on "git --shallow-file" without a value

On Wed, Aug 12, 2026 at 1:22 PM Patrick Steinhardt <[email protected]> wrote:
>
> On Tue, Aug 11, 2026 at 02:14:46PM +0200, Christian Couder wrote:
> > diff --git a/git.c b/git.c
> > index e5f1811b6b..96df15b5cd 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -304,11 +304,15 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
> >                       if (envchanged)
> >                               *envchanged = 1;
> >               } else if (!strcmp(cmd, "--shallow-file")) {
> > -                     (*argv)++;
> > -                     (*argc)--;
> > -                     setenv(GIT_SHALLOW_FILE_ENVIRONMENT, (*argv)[0], 1);
> > +                     if (*argc < 2) {
> > +                             fprintf(stderr, _("no file given for '%s' option\n" ), "--shallow-file");
> > +                             usage(git_usage_string);
>
> Should we maybe condense this into a single line?
>
>     usage(_("no file given for '%s' option\n")), "--shallow-file")
>
> I think that also printing the usage string is only distracting and
> doesn't really give the user a lot of extra context.

The goal of this patch is to fix the bug by using the same code as the
other options that can be passed a value like "--git-dir",
"--namespace", "--work-tree", and so on. Now all these options use the
same pattern for the error message:

git grep -A3 'if (\*argc < 2)' git.c
git.c:                  if (*argc < 2) {
git.c-                          fprintf(stderr, _("no directory given
for '%s' option\n" ), "--git-dir");
git.c-                          usage(git_usage_string);
git.c-                  }
--
git.c:                  if (*argc < 2) {
git.c-                          fprintf(stderr, _("no namespace given
for --namespace\n" ));
git.c-                          usage(git_usage_string);
git.c-                  }
--
git.c:                  if (*argc < 2) {
git.c-                          fprintf(stderr, _("no directory given
for '%s' option\n" ), "--work-tree");
git.c-                          usage(git_usage_string);
git.c-                  }
--
git.c:                  if (*argc < 2) {
git.c-                          fprintf(stderr, _("-c expects a
configuration string\n" ));
git.c-                          usage(git_usage_string);
git.c-                  }
--
git.c:                  if (*argc < 2) {
git.c-                          fprintf(stderr, _("no config key given
for --config-env\n" ));
git.c-                          usage(git_usage_string);
git.c-                  }
--
git.c:                  if (*argc < 2) {
git.c-                          fprintf(stderr, _("no directory given
for '%s' option\n" ), "-C");
git.c-                          usage(git_usage_string);
git.c-                  }
--
git.c:                  if (*argc < 2) {
git.c-                          fprintf(stderr, _("no attribute source
given for --attr-source\n" ));
git.c-                          usage(git_usage_string);
git.c-                  }

So I don't think it makes sense for "--shallow-file" to not be
consistent with these other options.

I could perhaps add a patch to the series to convert all of these to
something like what you suggest, but it could also be done in a
separate patch series by someone else.

Anyway thanks for reviewing this patch.