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 <p...@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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to