From 4df5e91867498f7c6ee4bf5a464a5c24dc89034b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 30 Aug 2016 23:41:22 -0400 Subject: [PATCH 1/2] error_errno: use constant return similar to error() 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 Signed-off-by: Junio C Hamano --- 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 49d4029b8d..24f0ec9986 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 1dad03fb5c..0efa3faf60 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]; From 3e1952ed96dde926ce38a8ab8803f9a410ecdae3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 30 Aug 2016 23:43:07 -0400 Subject: [PATCH 2/2] color_parse_mem: initialize "struct color" temporary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- color.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/color.c b/color.c index 8f85153d0d..c809548991 100644 --- 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])) {