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

From: Brandon Williams <[email-masked]>
Date: Fri, 9 Jun 2017 07:48:13 -0700

On 06/09, Ævar Arnfjörð Bjarmason wrote:
> On Fri, Jun 9, 2017 at 12:12 PM, Martin Ågren <[email-masked]> wrote:
> > On 9 June 2017 at 10:53, Ævar Arnfjörð Bjarmason <[email-masked]> 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.

It also couldn't hurt to add a comment to the macro definition
explaining that side-effect operators would be broken.

> 
> 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.

-- 
Brandon Williams