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

[Jeff King]

On Wed, Aug 31, 2016 at 12:55:01PM -0700, Junio C Hamano wrote:

> Interesting.  Here is for "gcc -Os" on top to appease gcc 4.8.4 that
> I probably am NOT going to apply.  These are all false positives.
> 
> The ones on config.c is the most curious as these two "ret" needs a
> false initialization, but the one that comes after them
> git_config_ulong() that has the same code structure does not get any
> warning, which made absolutely no sense to me.

Yeah, I'd agree that is really odd. I wondered if perhaps the signedness
of the argument mattered (e.g., if we were somehow provoking undefined
behavior which caused the compiler to make some assumption), but I just
don't see it.

>  builtin/update-index.c | 2 +-
>  config.c   | 4 ++--
>  diff.c | 2 +-
>  fast-import.c  | 1 +
>  4 files changed, 5 insertions(+), 4 deletions(-)

FWIW, all but the fast-import one have gone away in gcc 6.2.0 (using
-Os).

For that one:

> diff --git a/fast-import.c b/fast-import.c
> index bf53ac9..abc4519 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1377,6 +1377,7 @@ static const char *get_mode(const char *str, uint16_t 
> *modep)
>   unsigned char c;
>   uint16_t mode = 0;
>  
> + *modep = 0;
>   while ((c = *str++) != ' ') {
>   if (c < '0' || c > '7')
>   return NULL;

The complaint actually comes from the caller, who doesn't realize that
modep will be set.

It pretty clearly seems to be a false positive, but I don't understand
it. If get_mode() is not inlined (or otherwise examined when considering
the caller), then it presumably should be treated as a block box that we
assume sets "modep".  And if it is inlined, then it's pretty obvious
that "modep" is initialized in any code path that does not return NULL,
and we have:

p = get_mode(p, );
if (!p)
die("Corrupt mode: %s", command_buf.buf);

in the caller (and "die" is marked as NORETURN). So it seems like a
pretty easy case to get right, and one that the compiler presumably gets
right elsewhere (otherwise we'd have a lot more of these false
positives).

Weird.

-Peff

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

[Junio C Hamano]

Jeff King  writes:

> 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

Interesting.  Here is for "gcc -Os" on top to appease gcc 4.8.4 that
I probably am NOT going to apply.  These are all false positives.

The ones on config.c is the most curious as these two "ret" needs a
false initialization, but the one that comes after them
git_config_ulong() that has the same code structure does not get any
warning, which made absolutely no sense to me.


 builtin/update-index.c | 2 +-
 config.c   | 4 ++--
 diff.c | 2 +-
 fast-import.c  | 1 +
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index ba04b19..a202612 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -832,7 +832,7 @@ static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
const struct option *opt, int unset)
 {
unsigned char sha1[20];
-   unsigned int mode;
+   unsigned int mode = 0;
const char *path;
 
if (!parse_new_style_cacheinfo(ctx->argv[1], , sha1, )) {
diff --git a/config.c b/config.c
index 0dfed68..52133aa 100644
--- a/config.c
+++ b/config.c
@@ -685,7 +685,7 @@ static void die_bad_number(const char *name, const char 
*value)
 
 int git_config_int(const char *name, const char *value)
 {
-   int ret;
+   int ret = 0;
if (!git_parse_int(value, ))
die_bad_number(name, value);
return ret;
@@ -693,7 +693,7 @@ int git_config_int(const char *name, const char *value)
 
 int64_t git_config_int64(const char *name, const char *value)
 {
-   int64_t ret;
+   int64_t ret = 0;
if (!git_parse_int64(value, ))
die_bad_number(name, value);
return ret;
diff --git a/diff.c b/diff.c
index c7e2605..1098198 100644
--- a/diff.c
+++ b/diff.c
@@ -995,7 +995,7 @@ static int find_word_boundaries(mmfile_t *buffer, regex_t 
*word_regex,
 static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out,
regex_t *word_regex)
 {
-   int i, j;
+   int i, j = 0;
long alloc = 0;
 
out->size = 0;
diff --git a/fast-import.c b/fast-import.c
index bf53ac9..abc4519 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1377,6 +1377,7 @@ static const char *get_mode(const char *str, uint16_t 
*modep)
unsigned char c;
uint16_t mode = 0;
 
+   *modep = 0;
while ((c = *str++) != ' ') {
if (c < '0' || c > '7')
return NULL;

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

[Jeff King]

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