On 16/05/17 04:02, Jeff King wrote: > 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. ;-) > 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