Re: [PATCH 7/7] submodule: implement `module_clone` as a builtin helper

From: Jeff King <peff@peff.net>
Date: Aug 21, 2015, 14:22 UTC
Message-ID: <20150821142212.GA26130@sigill.intra.peff.net>
In-Reply-To: <1439857323-21048-8-git-send-email-sbeller@google.com>

On Mon, Aug 17, 2015 at 05:22:03PM -0700, Stefan Beller wrote:

Show 6 quoted lines

> +static int module_clone(int argc, const char **argv, const char *prefix)
> [...]
> +	/* Redirect the worktree of the submodule in the superprojects config */
> +	if (!is_absolute_path(sm_gitdir)) {
> +		char *s = (char*)sm_gitdir;
> +		strbuf_addf(&sb, "%s/%s", xgetcwd(), sm_gitdir);

Coverity noticed that this is a leak; xgetcwd actually returns a newly allocated buffer. I think you just want:

  if (strbuf_getcwd(&sb))
	die_errno("unable to get current working directory");
  strbuf_addf("/%s", sm_gitdir);

> +		sm_gitdir = strbuf_detach(&sb, NULL);
> +		strbuf_reset(&sb);

This reset is a noop after you've detached. It's not technically documented, but I do not think it is reasonable for it to work any other way. Maybe strbuf.h should make that promise.

> +	strbuf_addf(&sb, "%s/%s", xgetcwd(), path);

Ditto on the xgetcwd issue here.

-Peff