Re: [PATCH v2] Simplify handling of setup_git_directory_gently() failure cases.

>
> Hi Erin,
>
> On Sat, 15 Dec 2018, Erin Dahlgren wrote:
>
> > diff --git a/setup.c b/setup.c
> > index 1be5037..e1a9e17 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -831,16 +831,6 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,
> >       return NULL;
> >  }
> >
> > -static const char *setup_nongit(const char *cwd, int *nongit_ok)
> > -{
> > -     if (!nongit_ok)
> > -             die(_("not a git repository (or any of the parent directories): %s"), DEFAULT_GIT_DIR_ENVIRONMENT);
> > -     if (chdir(cwd))
> > -             die_errno(_("cannot come back to cwd"));
> > -     *nongit_ok = 1;
> > -     return NULL;
> > -}
> > -
> >  static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_len)
> >  {
> >       struct stat buf;
> > @@ -1097,18 +1087,20 @@ const char *setup_git_directory_gently(int *nongit_ok)
> >               prefix = setup_bare_git_dir(&cwd, dir.len, &repo_fmt, nongit_ok);
> >               break;
> >       case GIT_DIR_HIT_CEILING:
> > -             prefix = setup_nongit(cwd.buf, nongit_ok);
> > -             break;
> > +             if (!nongit_ok)
> > +                     die(_("not a git repository (or any of the parent directories): %s"),
> > +                                     DEFAULT_GIT_DIR_ENVIRONMENT);
>
> I am terribly sorry to bother you about formatting issues (in my mind, it
> is quite an annoying thing that we still have no fully automatic way to
> format Git's source code according to Git's preferred coding style, but
> there you go...): this `DEFAULT_GIT_DIR_ENVIRONMENT` should be aligned
> with the first parameter of `die()`, i.e.
>
> +               if (!nongit_ok)
> +                       die(_("not a git repository (or any of the parent directories): %s"),
> +                           DEFAULT_GIT_DIR_ENVIRONMENT);
>
> > +             *nongit_ok = 1;
> > +             strbuf_release(&dir);
> > +             return NULL;
> >       case GIT_DIR_HIT_MOUNT_POINT:
> > -             if (nongit_ok) {
> > -                     *nongit_ok = 1;
> > -                     strbuf_release(&cwd);
> > -                     strbuf_release(&dir);
> > -                     return NULL;
> > -             }
> > -             die(_("not a git repository (or any parent up to mount point %s)\n"
> > -                   "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
> > -                 dir.buf);
> > +             if (!nongit_ok)
> > +                     die(_("not a git repository (or any parent up to mount point %s)\n"
> > +                           "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
> > +                                     dir.buf);
>
> Likewise, `dir.buf` should be aligned with the `_` two lines above.

>
> Otherwise I think this patch is good to go!
>
> Thank you,
> Johannes
>
> > +             *nongit_ok = 1;
> > +             strbuf_release(&dir);
> > +             return NULL;
> >       default:
> >               BUG("unhandled setup_git_directory_1() result");
> >       }
> > --
> > 2.7.4
> >
> >