{"thread":{"id":"45975","subject":"[PATCH] usage: fix a sparse 'redeclared with different type' error","startedAt":"2017-05-16T01:11:49Z","lastAt":"2017-05-23T01:03:03Z","messageCount":4,"participants":["Ramsay Jones","Jeff King","Luc Van Oostenryck"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"319860","messageId":"52ef158a-0719-c71a-b72b-4961cf8f96d5@ramsayjones.plus.com","threadId":"45975","inReplyTo":null,"subject":"[PATCH] usage: fix a sparse 'redeclared with different type' error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-05-16T01:11:40Z","receivedAt":"2017-05-16T01:11:49Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSigned-off-by: Ramsay Jones \n---\n\nHi Jeff,\n\nIf you need to re-roll your 'jk/bug-to-abort' branch, could you please\nsquash this into the relevant patch (commit d8193743e0 \"usage.c: add\nBUG() function\", 12-05-2017).\n\n[Just FYI, sparse complains thus:\n usage.c:212:6: error: symbol 'BUG_fl' redeclared with different type\n (originally declared at git-compat-util.h:1076) - different modifiers\n]\n\nThanks!\n\nATB,\nRamsay Jones\n\n usage.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/usage.c b/usage.c\nindex 545136734..918b539bc 100644\n--- a/usage.c\n+++ b/usage.c\n@@ -209,6 +209,7 @@ static NORETURN void BUG_vfl(const char *file, int line, const char *fmt, va_lis\n }\n \n #ifdef HAVE_VARIADIC_MACROS\n+__attribute__((format (printf, 3, 4))) NORETURN\n void BUG_fl(const char *file, int line, const char *fmt, ...)\n {\n \tva_list ap;\n@@ -217,6 +218,7 @@ void BUG_fl(const char *file, int line, const char *fmt, ...)\n \tva_end(ap);\n }\n #else\n+__attribute__((format (printf, 1, 2))) NORETURN\n void BUG(const char *fmt, ...)\n {\n \tva_list ap;\n-- \n2.13.0\n"},{"id":"319867","messageId":"20170516030200.fivl7p5iukfxk2jb@sigill.intra.peff.net","threadId":"45975","inReplyTo":"52ef158a-0719-c71a-b72b-4961cf8f96d5@ramsayjones.plus.com","subject":"Re: [PATCH] usage: fix a sparse 'redeclared with different type' error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-16T03:02:00Z","receivedAt":"2017-05-16T03:02:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 16, 2017 at 02:11:40AM +0100, Ramsay Jones wrote:\n\n> \n> If you need to re-roll your 'jk/bug-to-abort' branch, could you please\n> squash this into the relevant patch (commit d8193743e0 \"usage.c: add\n> BUG() function\", 12-05-2017).\n> \n> [Just FYI, sparse complains thus:\n> usage.c:212:6: error: symbol 'BUG_fl' redeclared with different type\n> (originally declared at git-compat-util.h:1076) - different modifiers\n> ]\n\nHmm. Our model here is the die() function, which gets noreturn and\nformat attributes in the declaration, but only noreturn at the\ndefinition.\n\nYour patch here adds both attributes to the definition:\n\n> +__attribute__((format (printf, 3, 4))) NORETURN\n> void BUG_fl(const char *file, int line, const char *fmt, ...)\n\nAnother possible model is trace_printf_key_fl(), which just has a format\nattribute at the declaration, and nothing at all in the definition.\n\nSo it seems like this doesn't matter for the format attribute, but does\nfor NORETURN. Weird. I wonder if it's specific to the attribute, or\nsomething about the way we hide it behind a macro.\n\nThere probably isn't a downside to repeating the format attribute, I\nguess. Except that I'm not sure what happens if the two ever got out of\nsync (gcc doesn't seem to complain, though in practice you'd probably\nchange the order or number of arguments at the same time, which is\nlikely to cause a mismatch).\n\n-Peff\n"},{"id":"319916","messageId":"22dd8932-3086-f50d-3620-7ab336270b84@ramsayjones.plus.com","threadId":"45975","inReplyTo":"20170516030200.fivl7p5iukfxk2jb@sigill.intra.peff.net","subject":"Re: [PATCH] usage: fix a sparse 'redeclared with different type' error","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-05-16T12:27:55Z","receivedAt":"2017-05-16T12:29:19Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 16/05/17 04:02, Jeff King wrote:\n> On Tue, May 16, 2017 at 02:11:40AM +0100, Ramsay Jones wrote:\n> \n>>\n>> If you need to re-roll your 'jk/bug-to-abort' branch, could you please\n>> squash this into the relevant patch (commit d8193743e0 \"usage.c: add\n>> BUG() function\", 12-05-2017).\n>>\n>> [Just FYI, sparse complains thus:\n>> usage.c:212:6: error: symbol 'BUG_fl' redeclared with different type\n>> (originally declared at git-compat-util.h:1076) - different modifiers\n>> ]\n> \n> Hmm. Our model here is the die() function, which gets noreturn and\n> format attributes in the declaration, but only noreturn at the\n> definition.\n\nYes, in this case, it's only noreturn that matters (sparse only\ncares about a subset of attributes).\n\n> Your patch here adds both attributes to the definition:\n> \n>> +__attribute__((format (printf, 3, 4))) NORETURN\n>> void BUG_fl(const char *file, int line, const char *fmt, ...)\n\nYes, my initial patch only added the NORETURN, but I decided it\ndidn't hurt to make the header and c-file 'match' in this case. ;-)\n\n> There probably isn't a downside to repeating the format attribute, I\n> guess. Except that I'm not sure what happens if the two ever got out of\n> sync (gcc doesn't seem to complain, though in practice you'd probably\n> change the order or number of arguments at the same time, which is\n> likely to cause a mismatch).\n\nIf you prefer, you can simply add the NORETURN, since that eliminates\nthe error message just as well.\n\nATB,\nRamsay Jones\n\n\n"},{"id":"320491","messageId":"CAExDi1QdoxZFLtU5ja_2QpdCKkaOd69uHg6QMapk05MzGdrybw@mail.gmail.com","threadId":"45975","inReplyTo":"20170516030200.fivl7p5iukfxk2jb@sigill.intra.peff.net","subject":"Re: [PATCH] usage: fix a sparse 'redeclared with different type' error","fromName":"Luc Van Oostenryck","fromEmail":"luc.vanoostenryck@gmail.com","sentAt":"2017-05-23T01:02:56Z","receivedAt":"2017-05-23T01:03:03Z","isPatch":true,"sender":{"key":"luc.vanoostenryck@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16168334?v=4"},"body":"On Tue, May 16, 2017 at 5:02 AM, Jeff King wrote:\n> On Tue, May 16, 2017 at 02:11:40AM +0100, Ramsay Jones wrote:\n>\n>>\n>> If you need to re-roll your 'jk/bug-to-abort' branch, could you please\n>> squash this into the relevant patch (commit d8193743e0 \"usage.c: add\n>> BUG() function\", 12-05-2017).\n>>\n>> [Just FYI, sparse complains thus:\n>> usage.c:212:6: error: symbol 'BUG_fl' redeclared with different type\n>> (originally declared at git-compat-util.h:1076) - different modifiers\n>> ]\n>\n> Hmm. Our model here is the die() function, which gets noreturn and\n> format attributes in the declaration, but only noreturn at the\n> definition.\n\nIt's because a deficiency of sparse: the format attribute is simply ignored\nwhile the NORETURN is treated as a 'modifiers' not much different than\n'const' or 'extern'.\nBut the real problem here with sparse is that while it indeed compare\nthe declaration with the definition, this is done not by checking compatibility\nbut by strict equality. In other words, the NORETURN, but also a 'static',\nin the declaration should be somehow inherited in a subsequent definition\nbut sparse doesn't do that yet.\n\n-- Luc Van Oostenryck\n"}]}