Re: [PATCH v4 11/12] refs: implement logic to migrate between ref storage formats

> On Mon, Jun 03, 2024 at 11:31:00AM +0200, Patrick Steinhardt wrote:
> 
> > +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: