Hi Stefan,
On Wed, 26 Apr 2017, Stefan Beller wrote:
> On Wed, Apr 26, 2017 at 1:20 PM, Johannes Schindelin
> <johannes.schindelin@xxxxxx> wrote:
> > Reported by Coverity.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > setup.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/setup.c b/setup.c
> > index 0309c278218..0320a9ad14c 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -748,7 +748,7 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,
> >
> > /* --work-tree is set without --git-dir; use discovered one */
> > if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
> > - const char *gitdir;
> > + static const char *gitdir;
>
> This alone doesn't fix the memleak if the code were called multiple times.
> As by having it static it will be init'd to NULL, we could introduce a
>
> static const char *gitdir; /* must live until the end of time because ... */
> if (!gitdir)
> die("BUG: called setup_bare_git_dir twice?");
>
> I tried looking into setup_explicit_git_dir that is called with gitdir here
> and it looks like it duplicates the memory in most cases, so maybe we
> can even get away with no static variable here and instead either inline
> the gitdir computation in the arguments of setup_explicit_git_dir
> or have a variable here that holds the returns of setup_explicit_git_dir
> such that we can free after?
setup_bare_git_directory() is called by setup_git_directory(), and that
latter function is not reentrant. If you call setup_git_directory()
multiple times, it will wreak havoc, as e.g. the second time the prefix
cannot be determined correctly because we chdir()ed up into the top-level
directory.
So I am not worried about this new static, it is essentially a singleton.
Ciao,
Dscho