Re: [PATCH 08/13] submodule: fix cwd leak in get_superproject_working_tree()

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Patrick,

On Wed, 1 Jul 2026, Patrick Steinhardt wrote:

> On Wed, Jul 01, 2026 at 07:04:26AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > diff --git a/submodule.c b/submodule.c
> > index fd91201a92..8ddeebd8af 100644
> > --- a/submodule.c
> > +++ b/submodule.c
> > @@ -2627,10 +2627,10 @@ int get_superproject_working_tree(struct strbuf *buf)
> >  		 * We might have a superproject, but it is harder
> >  		 * to determine.
> >  		 */
> > -		return 0;
> > +		goto out;
> >  
> >  	if (!strbuf_realpath(&one_up, "../", 0))
> > -		return 0;
> > +		goto out;
> >  
> >  	subpath = relative_path(cwd, one_up.buf, &sb);
> >  	strbuf_release(&one_up);
> > @@ -2693,6 +2693,10 @@ int get_superproject_working_tree(struct strbuf *buf)
> >  		die(_("ls-tree returned unexpected return code %d"), code);
> >  
> >  	return ret;
> > +
> > +out:
> > +	free(cwd);
> > +	return 0;
> >  }
> 
> Okay. This is fine, but it feels a bit fragile as we also have a call to
> `free(cwd)` a bit further up. So if somebody were to add a `goto out`
> after that call we'd have a double free. Makes me wonder whether we want
> to have a single exit path for the complete function and then drop the
> other call to free(3p).

Agreed. In v2 the function has a single exit path: all late returns
fall through to the `out:` label, which additionally releases `sb`
and `one_up`.

A side effect worth noting is that consolidation also closes a latent
leak the original had on the `strbuf_realpath(&one_up, "../", 0)`
failure path. `strbuf_realpath_1()` calls `strbuf_reset(resolved)` on
error, which does not free the backing buffer, so `one_up` could
carry a residual allocation that the previous shape never released.

Ciao,
Johannes