Re: [PATCH v3 2/2] setup: fix memory leaks with `struct repository_format`

From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Jeff King <peff@peff.net>
Cc: "Martin Ågren" <martin.agren@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH v3 2/2] setup: fix memory leaks with `struct repository_format`
Date: Thu, 24 Jan 2019 00:14:51 +0000	[thread overview]
Message-ID: <20190124001450.GR423984@genre.crustytoothpaste.net> (raw)
In-Reply-To: <20190123055704.GA19601@sigill.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]

On Wed, Jan 23, 2019 at 12:57:05AM -0500, Jeff King wrote:
> This uses designated initializers, which is a C99-ism, but one we've
> used previously and feel confident in. But...
> 
> > +void clear_repository_format(struct repository_format *format)
> > +{
> > +	string_list_clear(&format->unknown_extensions, 0);
> > +	free(format->work_tree);
> > +	free(format->partial_clone);
> > +	*format = (struct repository_format)REPOSITORY_FORMAT_INIT;
> > +}
> 
> ...this uses that expression not as an initializer, but as a compound
> literal. That's also C99, but AFAIK it's the first usage in our code
> base. I don't know if it will cause problems or not.
> 
> The "old" way to do it is:
> 
>   struct repository_format foo = REPOSITORY_FORMAT_INIT;
>   memcpy(format, &foo, sizeof(foo));
> 
> Given how simple it is to fix if it turns out to be a problem, I'm OK
> including it as a weather balloon.

It's my understanding that MSVC doesn't support this construct. If we
care about supporting MSVC, then we need to write it without the
compound literal. MSVC doesn't support any C99 feature that is not also
in C++, unfortunately.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]