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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]