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:
>
> > +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
Attachment:
signature.asc
Description: PGP signature
