Re: [PATCH 2/2] *.[ch] refactoring: make use of the freez() wrapper

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 9, 2017, 10:58 UTC
Message-ID: <CACBZZX7U2SO11VZ43duLco1BhgM_XDzNTtrjcknjhijRkaeuxw@mail.gmail.com>
In-Reply-To: <CAN0heSo_S+_dut8RgDzVxffPZOzSTLwahsKrn7KG8aLvnrPbAw@mail.gmail.com>

On Fri, Jun 9, 2017 at 12:12 PM, Martin Ågren <martin.agren@gmail.com> wrote:

Show 16 quoted lines

> On 9 June 2017 at 10:53, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> Replace occurrences of `free(p); p = NULL` with `freez(p)`. This
>> introduces no functional changes, but cuts the number of lines spent
>> on this cleanup in half.
>
> It's even better than that. ;)
>
>>  48 files changed, 97 insertions(+), 197 deletions(-)
>
> The difference is in builtin/am.c where some empty lines are removed
> in am_next(), so no need to be alarmed.
>
> The macro would be dangerous with things like "freez(ptr++)" but I
> couldn't find any such side-effects. In hindsight, I guess your commit
> message says as much since using "ptr++" for "p" would already be a
> bug.

Yes, although perhaps we should call this FREEZ() or GIT_FREEZ() instead of freez() to make it clear that it's a macro.

> I have no idea whether this conflicts with other topics, or any
> opinion on the best strategy for doing the conversion (all-at-once or
> "while we're here").

It has no conflicts with pu, so that's something, and passes all tests with & without that merge.