Subject: Re: [RFC PATCH 01/15] remote.c: don't dereference NULL in freeing loop

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sat, 04 Jun 2022 18:20:54 +0200

On Sat, Jun 04 2022, Phillip Wood wrote:

>> On 03 June 2022 at 22:07 René Scharfe <[email-masked]> wrote:
>> 
>> 
>> Am 03.06.22 um 20:37 schrieb Ævar Arnfjörð Bjarmason:
>> > Fix a bug in fd3cb0501e1 (remote: move static variables into
>> > per-repository struct, 2021-11-17) where we'd free(remote->pushurl[i])
>> > after having NULL'd out remote->pushurl. itself.
>> >
>> > While we're at it let's get rid of the redundant braces per the
>> > CodingGuidelines, which also serves to show in the diff context that
>> > we were doing a FREE_AND_NULL(remote->pushurl) afterwards too, let's
>> > keep that one.
>> 
>> The extended context is helping, but the brace removal makes this change
>> harder to read.
>
> Indeed, a small style fix in a larger change is one thing but here at
> least 80% of the changed lines are unrelated to the bugfix. I'm afraid
> my heart has started to sink when I see the phrase "while we're at it"
> in a commit message.

I'm happy to omit it, I thought it was helpful to force the context to
be shown, will change that.