threads / patch / 45975

patchusage: fix a sparse 'redeclared with different type' error

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

## tl;dr

4 messages between May 16, 2017 and May 23, 2017. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Ramsay Jones· May 16, 2017, 01:11 UTC · lore

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Jeff,

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
]

Thanks!

ATB, Ramsay Jones

 usage.c | 2 ++
 1 file changed, 2 insertions(+)

Show changes to usage.c +2 −0

diff --git a/usage.c b/usage.c
index 545136734..918b539bc 100644
--- a/usage.c
+++ b/usage.c
@@ -209,6 +209,7 @@ static NORETURN void BUG_vfl(const char *file, int line, const char *fmt, va_lis
 }
 
 #ifdef HAVE_VARIADIC_MACROS
+__attribute__((format (printf, 3, 4))) NORETURN
 void BUG_fl(const char *file, int line, const char *fmt, ...)
 {
 	va_list ap;
@@ -217,6 +218,7 @@ void BUG_fl(const char *file, int line, const char *fmt, ...)
 	va_end(ap);
 }
 #else
+__attribute__((format (printf, 1, 2))) NORETURN
 void BUG(const char *fmt, ...)
 {
 	va_list ap;

-- 
2.13.0

Jeff King· May 16, 2017, 03:02 UTC · re: Ramsay Jones · lore

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

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

Ramsay Jones· May 16, 2017, 12:27 UTC · re: Jeff King · lore

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

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

Luc Van Oostenryck· May 23, 2017, 01:02 UTC · re: Jeff King · lore

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

On Tue, May 16, 2017 at 5:02 AM, Jeff King <peff@peff.net> 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.

It's because a deficiency of sparse: the format attribute is simply ignored while the NORETURN is treated as a 'modifiers' not much different than 'const' or 'extern'. But the real problem here with sparse is that while it indeed compare the declaration with the definition, this is done not by checking compatibility but by strict equality. In other words, the NORETURN, but also a 'static', in the declaration should be somehow inherited in a subsequent definition but sparse doesn't do that yet.

-- Luc Van Oostenryck