Re: [PATCH v3 02/11] reftable: fix resource leak in error path

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 13, 2021, 22:10 UTC
Message-ID: <xmqqzgp49mwn.fsf@gitster.g>
In-Reply-To: <CAFQ2z_MrSEExScfBPku1uTZQgBYyk-B08U98zs5P7LNdLphutg@mail.gmail.com>

Han-Wen Nienhuys <hanwen@google.com> writes:

Show 15 quoted lines

> On Mon, Dec 13, 2021 at 5:21 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Other things in the codebase don't check for NULL before feeding things
>> to reftable_free(), and our own free() has a coccicheck rule to catch
>> this sort of code, we should probably add reftable_free to that list...
>
> Thanks, changed.
>
>> > +     struct reftable_block_source source = { NULL };
>>
>> Nit: It doesn't matter for semantics, but usually we use "{ 0 }", and
>> your 1/11 does too. Would be better to do that here for consistency.
>
> I got a past review where someone complained about this. I don't mind
> either way, but would rather not flipflop.

The last part is important.

For initializers that show the value for the first member in {} to mean "everything is zero-initialized", only because the language does not allow us to write

	struct foo var = {};

we historically used { NULL } for pointers and { 0 } for integral types (primarily because auto checkers like sparse did not like us to write a NULL pointer as 0), but we started preferring { 0 } as more recent versions of checkers understand it as an idiom, and we can freely reorder the struct members if we consistently spell the "everything is zero-initialized" that way.

So, let's use { 0 } here, too.

Thanks.