color_parse_mem: initialize "struct color" temporary
authorJeff King <peff@peff.net>
Wed, 31 Aug 2016 03:43:07 +0000 (23:43 -0400)
committerJunio C Hamano <gitster@pobox.com>
Wed, 31 Aug 2016 18:11:55 +0000 (11:11 -0700)
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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
color.c
diff --git a/color.c b/color.c
index 8f85153d0d2c1f4bf9340ced9371797eb3c92de9..c809548991d3b247e7e5476030edb0d88d9179f9 100644 (file)
--- a/color.c
+++ b/color.c
@@ -200,7 +200,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])) {