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

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: May 16, 2017, 12:27 UTC
Message-ID: <22dd8932-3086-f50d-3620-7ab336270b84@ramsayjones.plus.com>
In-Reply-To: <20170516030200.fivl7p5iukfxk2jb@sigill.intra.peff.net>

On 16/05/17 04:02, Jeff King wrote:

Show 15 quoted lines

> On Tue, May 16, 2017 at 02:11:40AM +0100, Ramsay Jones wrote:
> 
>>
>> 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.

Yes, in this case, it's only noreturn that matters (sparse only cares about a subset of attributes).

> 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, ...)

Yes, my initial patch only added the NORETURN, but I decided it didn't hurt to make the header and c-file 'match' in this case. ;-)

Show 5 quoted lines

> 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).

If you prefer, you can simply add the NORETURN, since that eliminates the error message just as well.

ATB, Ramsay Jones