From: Jeff King <peff@peff.net>
Date: Tue, 01 Sep 2015 19:05:06 GMT
Subject: Re: [PATCHv4 3/3] submodule: implement `clone` as a builtin helper
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:

> > +       /* 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:

> 
> > +               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