Re: [PATCHv4 3/3] submodule: implement `clone` as a builtin helper

From: Jeff King <peff@peff.net>
Date: Sep 1, 2015, 19:05 UTC
Message-ID: <20150901190506.GA8254@sigill.intra.peff.net>
In-Reply-To: <CAPig+cRxQdquKZXdmpiZjj8JFB2wdCE4UcbW6qYUwvtZsVw=ug@mail.gmail.com>

On Tue, Sep 01, 2015 at 02:52:54PM -0400, Eric Sunshine wrote:

Show 10 quoted lines

> > +       /* Redirect the worktree of the submodule in the superproject's config */
> > +       if (strbuf_getcwd(&sb))
> > +               die_errno(_("unable to get current working directory"));
> > +
> > +       if (!is_absolute_path(sm_gitdir)) {
> > +               if (strbuf_getcwd(&sb))
> > +                       die_errno(_("unable to get current working directory"));
> 
> Why does this need to call getcwd() on 'sb' when it was called
> immediately above the conditional and its value hasn't changed?

Even weirder, the next code is:

Show 8 quoted lines

> 
> > +               strbuf_addf(&sb, "/%s", sm_gitdir);
> > +               free(sm_gitdir);
> > +               sm_gitdir = strbuf_detach(&sb, NULL);
> > +       }
> > +
> > +
> > +       strbuf_addf(&sb, "/%s", path);

So if it _was_ an absolute path, we are adding "/$path" to nothing (having just detached the strbuf in the conditional above). That seems weird.

-Peff