Re: [PATCH 1/2] introduce "banned function" list

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 19, 2018, 21:11 UTC
Message-ID: <CAPig+cSErDoZ5XFjfJsqfEKZ6PSKfOos=9HW-7FDXU1XEtuktA@mail.gmail.com>
In-Reply-To: <20180719203901.GA8079@sigill.intra.peff.net>

On Thu, Jul 19, 2018 at 4:39 PM Jeff King <peff@peff.net> wrote:

Show 15 quoted lines

> [...]
> Let's start by banning strcpy() and sprintf(). It's not
> impossible to use these correctly, but it's easy to do so
> incorrectly, and there's always a better option.
> [...]
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/banned.h b/banned.h
> @@ -0,0 +1,19 @@
> +/*
> + * This header lists functions that have been banned from our code base,
> + * because they're too easy to misuse (and even if used correctly,
> + * complicate audits). Including this header turns them into compile-time
> + * errors.
> + */

When the above talks about "including this header", the implication is that it must be included _after_ the system header(s) which declare the banned functions. I wonder if that requirement should be stated here explicitly.

(Probably not worth a re-roll.)

Show 9 quoted lines

> +#define BANNED(func) sorry_##func##_is_a_banned_function()
> +
> +#define strcpy(x,y) BANNED(strcpy)
> diff --git a/git-compat-util.h b/git-compat-util.h
> @@ -1239,4 +1239,6 @@ extern void unleak_memory(const void *ptr, size_t len);
>  #define UNLEAK(var) do {} while (0)
>  #endif
>
> +#include "banned.h"