git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH/RFC 3/3] silence some -Wuninitialized warnings around errors

From
Jeff King <peff@peff.net>
Date
Dec 14, 2012, 22:13 UTC
Message-ID
<20121214221317.GC19677@sigill.intra.peff.net>
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
Previous: Jeff KingNext: Nguyen Thai Ngoc Duy
Message 5 of 12 in “compiling git with gcc -O3 -Wuninitialized”
  1. 0/3 compiling git with gcc -O3 -WuninitializedJeff King, Dec 14, 2012
  2. 1/3 remote-testsvn: fix unitialized variableJeff King, Dec 14, 2012
  3. Florian AchleitnerDec 15, 2012
  4. 2/3 inline error functions with constant returnsJeff King, Dec 14, 2012
  5. 3/3 silence some -Wuninitialized warnings around errorsJeff King, Dec 14, 2012
  6. Nguyen Thai Ngoc DuyDec 15, 2012
  7. Jeff KingDec 15, 2012
  8. Johannes SixtDec 15, 2012
  9. Jeff KingDec 15, 2012
  10. 0/2 compiling git with gcc -O3 -WuninitializedJeff King, Dec 15, 2012
  11. 1/2 make error()'s constant return value more visibleJeff King, Dec 15, 2012
  12. 2/2 silence some -Wuninitialized false positivesJeff King, Dec 15, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.