On Thu, Oct 15, 2020 at 10:15:42AM -0700, Junio C Hamano wrote: > > As others noted, gcc 10 actually does complain about this. And we can > > easily stick -fno-common into the DEVELOPER knobs, if it's something we > > want to catch (I had actually forgotten it wasn't the default). > > Yup, that is a good thing to do regardless. I am mostly interested > in seeing "extern" in front of all extern decls (not defns) from > human readers' point of view, though. OK, patch incoming. > > I see you picked up my patch as jk/report-fn-typedef, but applied it > > directly on v2.28, and not on top of your "drop these extra externs" > > patch. That makes sense if we're not going to remove them, but then your > > conflict resolution shows my patch as removing them. :) > > I think the patch rearranged that way shows why the new typedef > shines. > > It makes it immediately recognisable that set_die_routine() and > friends are functions, not pointers to functions, and because the > reason why "extern" was in front of them was because the last person > who "cleaned up" the header failed to recoginise that they are > functions without the help from this new typedef. > > All of that depends on the fact that everybody understands that the > current rule is not to write "extern" in front of external > declaration of functions, so perhaps we may want to update the log > message to mention why "extern" were removed from these two. I added: Note that this also removes the "extern" from these declarations to match the surrounding functions. They were missed in 554544276a (*.[ch]: remove extern from function declarations using spatch, 2019-04-29) presumably because of the unusual syntax. which I think clarifies it. Here's that patch re-rolled, plus the DEVELOPER one. [1/2]: usage: define a type for a reporting function [2/2]: config.mak.dev: build with -fno-common config.mak.dev | 1 + git-compat-util.h | 12 +++++++----- usage.c | 18 +++++++++--------- 3 files changed, 17 insertions(+), 14 deletions(-) -Peff