On Wed, Sep 20, 2017 at 10:45:05AM +0900, Junio C Hamano wrote:

> >> > +#else
> >> > +#define UNLEAK(var)
> >> 
> >> I think this should be defined to be something (for example, "do {}
> >> while (0)"), at least so that we have compiler errors when UNLEAK(var)
> >> is used incorrectly (for example, without the semicolon) when
> >> SUPPRESS_ANNOTATED_LEAKS is not defined.
> >
> > Seems reasonable.
> 
> Hmph, I am not so sure about this one.  But I agree that the
> semicolon must go.

I thought we had run into some issues with a compiler or linter
complaining about null statements before. But they _are_ pretty common,
so maybe I'm mis-remembering (or maybe it was something like the if/else
conditional you showed earlier).

At any rate, I think Jonathan's point is that writing:

  UNLEAK(foo)

will silently pass in a normal build, and only much later will somebody
run a leak-checking build and see the compile error.

Of course people adding UNLEAK()s are likely to be compiling leak-check
builds to confirm that they've silenced the warning, so maybe it's not
that important a case to catch.

-Peff

Reply via email to