[PATCH 0/2] squelch some "gcc -O3 -Wmaybe-uninitialized" warnings

I happened to be compiling git with -O3 today, and noticed we generate
some warnings about uninitialized variables (I actually compile with
-Wall, but the only false positives I saw were these).

Here are patches to squelch them.

  [1/2]: error_errno: use constant return similar to error()
  [2/2]: color_parse_mem: initialize "struct color" temporary

-Peff

Commit e208f9c (make error()'s constant return value more
visible, 2012-12-15) introduced some macro trickery to make
the constant return from error() more visible to callers,
which in turn can help gcc produce better warnings (and
possibly even better code).

Later, fd1d672 (usage.c: add warning_errno() and
error_errno(), 2016-05-08) introduced another variant, and
subsequent commits converted some uses of error() to
error_errno(), losing the magic from e208f9c for those
sites.

As a result, compiling vcs-svn/svndiff.c with "gcc -O3"
produces -Wmaybe-uninitialized false positives (at least
with gcc 6.2.0). Let's give error_errno() the same
treatment, which silences these warnings.

Signed-off-by: Jeff King <redacted>
---
 git-compat-util.h | 1 +
 usage.c           | 1 +
 2 files changed, 2 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index db89ba7..2ad45b3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -436,6 +436,7 @@ static inline int const_error(void)
 	return -1;
 }
 #define error(...) (error(__VA_ARGS__), const_error())
+#define error_errno(...) (error_errno(__VA_ARGS__), const_error())
 #endif
 
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));

diff --git a/usage.c b/usage.c
index 1dad03f..0efa3fa 100644
--- a/usage.c
+++ b/usage.c

@@ -148,6 +148,7 @@ void NORETURN die_errno(const char *fmt, ...)
 	va_end(params);
 }
 
+#undef error_errno
 int error_errno(const char *fmt, ...)
 {
 	char buf[1024];

-- 
2.10.0.rc2.125.gcfb3d08

Compiling color.c with gcc 6.2.0 using -O3 produces some
-Wmaybe-uninitialized false positives:

    color.c: In function ‘color_parse_mem’:
    color.c:189:10: warning: ‘bg.blue’ may be used uninitialized in this function [-Wmaybe-uninitialized]
       out += xsnprintf(out, len, "%c8;2;%d;%d;%d", type,
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          c->red, c->green, c->blue);
          ~~~~~~~~~~~~~~~~~~~~~~~~~~
    color.c:208:15: note: ‘bg.blue’ was declared here
      struct color bg = { COLOR_UNSPECIFIED };
                   ^~
    [ditto for bg.green, bg.red, fg.blue, etc]

This is doubly confusing, because the declaration shows it
being initialized! Even though we do not explicitly
initialize the color components, an incomplete initializer
sets the unmentioned members to zero.

What the warning doesn't show is that we later do this:

  struct color c;
  if (!parse_color(&c, ...)) {
          if (fg.type == COLOR_UNSPECIFIED)
                fg = c;
          ...
  }

gcc is clever enough to realize that a struct assignment
from an uninitialized variable taints the destination. But
unfortunately it's _not_ clever enough to realize that we
only look at those members when type is set to COLOR_RGB, in
which case they are always initialized.

With -O2, gcc does not look into parse_color() and must
assume that "c" emerges fully initialized. With -O3, it
inlines parse_color(), and learns just enough to get
confused.

We can silence the false positive by initializing the
temporary "c". This also future-proofs us against
violating the type assumptions (the result would probably
still be buggy, but in a deterministic way).

Signed-off-by: Jeff King <redacted>
---
Of course it's possible that I am wrong and gcc is right, but I just
don't see it (and that was the real reason I dug; I don't care _that_
much about -O3 warnings, but I wanted to see if gcc had found a real
bug).

 color.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/color.c b/color.c
index 81c2676..1b95e6b 100644
--- a/color.c
+++ b/color.c

@@ -215,7 +215,7 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 	/* [fg [bg]] [attr]... */
 	while (len > 0) {
 		const char *word = ptr;
-		struct color c;
+		struct color c = { COLOR_UNSPECIFIED };
 		int val, wordlen = 0;
 
 		while (len > 0 && !isspace(word[wordlen])) {

-- 
2.10.0.rc2.125.gcfb3d08