Re: [PATCH v2 10/10] add UNLEAK annotation for reducing leak false positives

From: Jeff King <peff@peff.net>
Date: Sep 19, 2017, 21:03 UTC
Message-ID: <20170919210300.cbrdjqor6xuwd7bs@sigill.intra.peff.net>
In-Reply-To: <20170919134552.7845f021@twelve2.svl.corp.google.com>

On Tue, Sep 19, 2017 at 01:45:52PM -0700, Jonathan Tan wrote:

> The following comments are assuming that we're going to standardize on
> UNLEAK(var); (with the semicolon).

Yeah, I assumed we would. We don't have to, since this really is sort-of magical, but I think the code looks better with it.

Show 9 quoted lines

> On Fri, 8 Sep 2017 02:38:41 -0400
> Jeff King <peff@peff.net> wrote:
> 
> > +#ifdef SUPPRESS_ANNOTATED_LEAKS
> > +extern void unleak_memory(const void *ptr, size_t len);
> > +#define UNLEAK(var) unleak_memory(&(var), sizeof(var));
> 
> I would feel better if the semicolon was omitted. I don't think it
> matters in this particular case, though.

You end up with a double semi-colon. Some linters might complain.

Show 7 quoted lines

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

I think both are worth doing, but the patch is already in next so we need to do it on top. Do you want to prepare a patch?

-Peff