From: Jonathan Nieder <jrnieder@gmail.com>
To: Stefan Beller <stefanbeller@googlemail.com>
Cc: git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH] Makefile: suppress false positive warnings of empty format string.
Date: Sun, 29 Sep 2013 12:00:17 -0700 [thread overview]
Message-ID: <20130929190017.GA2482@elie.Belkin> (raw)
In-Reply-To: <1380456534-7582-1-git-send-email-stefanbeller@googlemail.com>
Hi,
Stefan Beller wrote:
> --- a/Makefile
> +++ b/Makefile
> @@ -349,7 +349,7 @@ 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
Thanks for taking this on. Two thoughts:
1) As Felipe mentioned, this isn't portable. Would it make sense to
make it conditional on the value of $(CC) or the output of
"$(CC) --version"?
2) I don't understand the value of -Wformat-zero-length at all. What
bug is it meant to prevent? Do you know if anyone has asked the
GCC maintainers to disable it from the default set of warnings in
-Wall, which would give us a bug number to point to?
3) Since we don't enable -Werror by default (which is really good ---
use of -Werror can be a fine development tool but is a terrible
default), the warning does not actually do much harm. When it
becomes harmful is when someone turns on -Werror for static
analysis purposes and is unable to move forward from there. Do we
document suggested pedantic compiler flags anywhere other than the
todo:Make script? Should we?
I suspect such a documentation fix would have more impact, since it
could encourage people to experiment with flags and to check their
code strictly even when warnings are not portable, without
significantly slowing down the normal build.
For reference, todo:Make suggests the following flags:
-Wpointer-arith -Woverflow -Wunused \
-Wno-pointer-to-int-cast -Werror \
-Wold-style-definition std=c99 -O2 \
-Wall -Wdeclaration-after-statement -Wno-format-zero-length -g
Hope that helps,
Jonathan
next prev parent reply other threads:[~2013-09-29 19:00 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-11 23:36 What's cooking in git.git (Jul 2013, #04; Thu, 11) Junio C Hamano
2013-09-29 12:08 ` [PATCH] Makefile: suppress false positive warnings of empty format string Stefan Beller
2013-09-29 15:07 ` Felipe Contreras
2013-09-29 15:23 ` Ramsay Jones
2013-09-29 19:00 ` Jonathan Nieder [this message]
2013-09-30 20:14 ` Jeff King
2013-09-30 21:26 ` Jonathan Nieder
2013-09-30 21:34 ` Jeff King
2013-09-30 21:38 ` Stefan Beller
2013-09-30 23:23 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130929190017.GA2482@elie.Belkin \
--to=jrnieder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=stefanbeller@googlemail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).