git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH/RFC 3/3] silence some -Wuninitialized warnings around errors
Date: Fri, 14 Dec 2012 17:13:18 -0500	[thread overview]
Message-ID: <20121214221317.GC19677@sigill.intra.peff.net> (raw)
In-Reply-To: <20121214220903.GA18418@sigill.intra.peff.net>

When git is compiled with "gcc -Wuninitialized -O3", some
inlined calls provide an additional opportunity for the
compiler to do static analysis on variable initialization.
For example, with two functions like this:

  int get_foo(int *foo)
  {
	if (something_that_might_fail() < 0)
		return error("unable to get foo");
	*foo = 0;
	return 0;
  }

  void some_fun(void)
  {
	  int foo;
	  if (get_foo(&foo) < 0)
		  return -1;
	  printf("foo is %d\n", foo);
  }

If get_foo() is not inlined, then when compiling some_fun,
gcc sees only that a pointer to the local variable is
passed, and must assume that it is an out parameter that
is initialized after get_foo returns.

However, when get_foo() is inlined, the compiler may look at
all of the code together and see that some code paths in
get_foo() do not initialize the variable. As a result, it
prints a warning. But what the compiler can't see is that
error() always returns -1, and therefore we know that either
we return early from some_fun, or foo ends up initialized,
and the code is safe.  The warning is a false positive.

By converting the error() call to:

  error("unable to get foo");
  return -1;

we explicitly make the compiler aware of the constant return
value, and it silences the warning.

Signed-off-by: Jeff King <peff@peff.net>
---
Not sure if we should do this or not. This does silence the warnings,
but it's kind of ugly.

 builtin/reflog.c  | 6 ++++--
 vcs-svn/svndiff.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index b3c9e27..c2ea9d3 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -494,8 +494,10 @@ static int parse_expire_cfg_value(const char *var, const char *value, unsigned l
 
 static int parse_expire_cfg_value(const char *var, const char *value, unsigned long *expire)
 {
-	if (!value)
-		return config_error_nonbool(var);
+	if (!value) {
+		config_error_nonbool(var);
+		return -1;
+	}
 	if (!strcmp(value, "never") || !strcmp(value, "false")) {
 		*expire = 0;
 		return 0;
diff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c
index 74c97c4..46e96f6 100644
--- a/vcs-svn/svndiff.c
+++ b/vcs-svn/svndiff.c
@@ -121,7 +121,8 @@ static int read_int(struct line_buffer *in, uintmax_t *result, off_t *len)
 		*len = sz - 1;
 		return 0;
 	}
-	return error_short_read(in);
+	error_short_read(in);
+	return -1;
 }
 
 static int parse_int(const char **buf, size_t *result, const char *end)
@@ -140,7 +141,8 @@ static int parse_int(const char **buf, size_t *result, const char *end)
 		*buf = pos + 1;
 		return 0;
 	}
-	return error("invalid delta: unexpected end of instructions section");
+	error("invalid delta: unexpected end of instructions section");
+	return -1;
 }
 
 static int read_offset(struct line_buffer *in, off_t *result, off_t *len)
-- 
1.8.0.2.4.g59402aa

  parent reply	other threads:[~2012-12-14 22:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-14 22:09 [PATCH/RFC 0/3] compiling git with gcc -O3 -Wuninitialized Jeff King
2012-12-14 22:11 ` [PATCH 1/3] remote-testsvn: fix unitialized variable Jeff King
2012-12-15 10:29   ` Florian Achleitner
2012-12-14 22:12 ` [PATCH/RFC 2/3] inline error functions with constant returns Jeff King
2012-12-14 22:13 ` Jeff King [this message]
2012-12-15  3:07 ` [PATCH/RFC 0/3] compiling git with gcc -O3 -Wuninitialized Nguyen Thai Ngoc Duy
2012-12-15 10:09   ` Jeff King
2012-12-15 10:49 ` Johannes Sixt
2012-12-15 11:09   ` Jeff King
2012-12-15 17:36     ` [PATCH/RFCv2 0/2] " Jeff King
2012-12-15 17:37       ` [PATCH 1/2] make error()'s constant return value more visible Jeff King
2012-12-15 17:42       ` [PATCH 2/2] silence some -Wuninitialized false positives 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=20121214221317.GC19677@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /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).