On Wed, Jun 05, 2024 at 06:03:18AM -0400, Jeff King wrote:

On Mon, Jun 03, 2024 at 11:31:00AM +0200, Patrick Steinhardt wrote:

quoted

+int repo_migrate_ref_storage_format(struct repository *repo,
+				    enum ref_storage_format format,
+				    unsigned int flags,
+				    struct strbuf *errbuf)
+{
[...]
+	new_gitdir = mkdtemp(xstrdup(buf.buf));
+	if (!new_gitdir) {
+		strbuf_addf(errbuf, "cannot create migration directory: %s",
+			    strerror(errno));
+		ret = -1;
+		goto done;
+	}

Coverity complains here of a leak of the xstrdup(). The return from
mkdtemp() should generally point to the same buffer we passed in, but if
it sees an error it will return NULL and the new heap buffer will be
lost.

Probably unlikely, but since you are on a leak-checking kick, I thought
I'd mention it. ;)

Since you have a writable strbuf already, maybe:

  new_gitdir = mkdtemp(buf.buf);
  if (!new_gitdir)
	...
  new_gitdir = strbuf_detach(&buf, NULL); /* same pointer, but now we own it */

Or since "buf" is not used for anything else, we could just leave it
attached to the strbuf. And probably give it a better name. Maybe:

I like that version, thanks!

Patrick