Re: [PATCH] difftool.c: mark a file-local symbol with static
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 1, 2016, 18:20 UTC
- Message-ID
- <xmqq60n3bjel.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20161201040234.3rnuttitneweedn5@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 11 quoted lines
> I don't have a preference on which direction we go, but yes, right now
> we are in an awkward middle ground. We should do one of:
>
> 1. Drop -Wno-format-zero-length from DEVELOPER_CFLAGS and make sure
> future patches to do not violate it.
>
> 2. Declare warning("") as OK.
>
> I still think the warning is silly, but (1) has value in that it
> produces the least surprise and annoyance to various people building
> Git.What is most awkward is that "make", with no customization out of box, suggests to use -Wall in CFLAGS and triggers the misguided warning, and DEVELOPER_CFLAGS, partly because it adds -Werror to turn warnings (including this misguided one) into errors, disables it with -Wno-format-zero-length. As a result:
- Casual builders that follow our suggestion get warnings.
- Developers do not notice their new code may make the "casual builder" experience worse.
That is totally backwards. That is probably what you meant by "the least surprise and annoyance"?
I also still think that any_printf_like_function("%s", "") looks silly. I know that we've already started moving in that direction and we stopped at a place we do not want to be in, but perhaps it was a mistake to move in that direction in the first place. I am tempted to say we should do something like the attached, but that may not fly well, as I suspect that -Wno-format-zero-length may be a lot more exotic than the -Wall compiler option. An obvious second best option would be to drop -Wall from the "everybody" CFLAGS and move it to DEVELOPER_CFLAGS instead.
diff --git a/Makefile b/Makefile index a379738db2..137c10e257 100644 --- a/Makefile +++ b/Makefile @@ -391,10 +391,9 @@ GIT-VERSION-FILE: FORCE # CFLAGS and LDFLAGS are for the users to override from the command line. -CFLAGS = -g -O2 -Wall +CFLAGS = -g -O2 -Wall -Wno-format-zero-length DEVELOPER_CFLAGS = -Werror \ -Wdeclaration-after-statement \ - -Wno-format-zero-length \ -Wold-style-definition \ -Woverflow \ -Wpointer-arith \