Re: [PATCH] usage: fix a sparse 'redeclared with different type' error

From: Jeff King <peff@peff.net>
Date: May 16, 2017, 03:02 UTC
Message-ID: <20170516030200.fivl7p5iukfxk2jb@sigill.intra.peff.net>
In-Reply-To: <52ef158a-0719-c71a-b72b-4961cf8f96d5@ramsayjones.plus.com>

On Tue, May 16, 2017 at 02:11:40AM +0100, Ramsay Jones wrote:

Show 9 quoted lines

> 
> If you need to re-roll your 'jk/bug-to-abort' branch, could you please
> squash this into the relevant patch (commit d8193743e0 "usage.c: add
> BUG() function", 12-05-2017).
> 
> [Just FYI, sparse complains thus:
>   usage.c:212:6: error: symbol 'BUG_fl' redeclared with different type
>   (originally declared at git-compat-util.h:1076) - different modifiers
> ]

Hmm. Our model here is the die() function, which gets noreturn and format attributes in the declaration, but only noreturn at the definition.

Your patch here adds both attributes to the definition:

> +__attribute__((format (printf, 3, 4))) NORETURN
>  void BUG_fl(const char *file, int line, const char *fmt, ...)

Another possible model is trace_printf_key_fl(), which just has a format attribute at the declaration, and nothing at all in the definition.

So it seems like this doesn't matter for the format attribute, but does for NORETURN. Weird. I wonder if it's specific to the attribute, or something about the way we hide it behind a macro.

There probably isn't a downside to repeating the format attribute, I guess. Except that I'm not sure what happens if the two ever got out of sync (gcc doesn't seem to complain, though in practice you'd probably change the order or number of arguments at the same time, which is likely to cause a mismatch).

-Peff