[PATCH 1/2] fast-import: Fix an gcc -Wuninitialized warning

Commit cbfd5e1c ("drop some obsolete "x = x" compiler warning hacks",
21-03-2013) removed a gcc hack that suppressed an "might be used
uninitialized" warning issued by older versions of gcc.

However, commit 3aa99df8 ('fast-import: clarify "inline" logic in
file_change_m', 21-03-2013) addresses an (almost) identical issue
(with very similar code), but includes additional code in it's
resolution. The solution used by this commit, unlike that used by
commit cbfd5e1c, also suppresses the -Wuninitialized warning on
older versions of gcc.

In order to suppress the warning (against the 'oe' symbol) in the
note_change_n() function, we adopt the same solution used by commit
3aa99df8.

Signed-off-by: Ramsay Jones <redacted>
---
 fast-import.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fast-import.c b/fast-import.c
index a0c2c2f..5f539d7 100644
--- a/fast-import.c
+++ b/fast-import.c

@@ -2465,6 +2465,7 @@ static void note_change_n(struct branch *b, unsigned char *old_fanout)
 		hashcpy(sha1, oe->idx.sha1);
 	} else if (!prefixcmp(p, "inline ")) {
 		inline_data = 1;
+		oe = NULL; /* not used with inline_data, but makes gcc happy */
 		p += strlen("inline");  /* advance to space */
 	} else {
 		if (get_sha1_hex(p, sha1))

-- 
1.8.2

On Tue, Mar 26, 2013 at 07:09:44PM +0000, Ramsay Jones wrote:

Commit cbfd5e1c ("drop some obsolete "x = x" compiler warning hacks",
21-03-2013) removed a gcc hack that suppressed an "might be used
uninitialized" warning issued by older versions of gcc.

However, commit 3aa99df8 ('fast-import: clarify "inline" logic in
file_change_m', 21-03-2013) addresses an (almost) identical issue
(with very similar code), but includes additional code in it's
resolution. The solution used by this commit, unlike that used by
commit cbfd5e1c, also suppresses the -Wuninitialized warning on
older versions of gcc.

In order to suppress the warning (against the 'oe' symbol) in the
note_change_n() function, we adopt the same solution used by commit
3aa99df8.

Yeah, they are essentially the same piece of code, so I don't mind this
change.  It is odd to me that gcc gets it right in one case but not the
other, but I think we are deep into the vagaries of the compiler's code
flow analysis here, and we cannot make too many assumptions.

Were you actually triggering this warning, and if so, on what version of
gcc? Or did the asymmetry just offend your sensibilities?

-Peff

Jeff King wrote:

On Tue, Mar 26, 2013 at 07:09:44PM +0000, Ramsay Jones wrote:

quoted

Commit cbfd5e1c ("drop some obsolete "x = x" compiler warning hacks",
21-03-2013) removed a gcc hack that suppressed an "might be used
uninitialized" warning issued by older versions of gcc.

However, commit 3aa99df8 ('fast-import: clarify "inline" logic in
file_change_m', 21-03-2013) addresses an (almost) identical issue
(with very similar code), but includes additional code in it's
resolution. The solution used by this commit, unlike that used by
commit cbfd5e1c, also suppresses the -Wuninitialized warning on
older versions of gcc.

In order to suppress the warning (against the 'oe' symbol) in the
note_change_n() function, we adopt the same solution used by commit
3aa99df8.

Yeah, they are essentially the same piece of code, so I don't mind this
change.  It is odd to me that gcc gets it right in one case but not the
other, but I think we are deep into the vagaries of the compiler's code
flow analysis here, and we cannot make too many assumptions.

Were you actually triggering this warning, and if so, on what version of
gcc? 

yes, with:
    gcc v3.4.4 (cygwin)
    gcc v4.1.2 (Linux)
    msvc v15.00.30729.01 (VC/C++ 2008 express edition)
no, with:
    gcc v4.4.0 (msysgit)
    clang 3.2 (Linux)
    tcc v0.9.26 (Linux)
[lcc can't compile git; I forget why exactly.]

      Or did the asymmetry just offend your sensibilities?

My sensibilities were, indeed, very offended! ;-)

ATB,
Ramsay Jones

On Thu, Mar 28, 2013 at 06:20:29PM +0000, Ramsay Jones wrote:

Jeff King wrote:

quoted

On Tue, Mar 26, 2013 at 07:09:44PM +0000, Ramsay Jones wrote:

quoted

Commit cbfd5e1c ("drop some obsolete "x = x" compiler warning hacks",
21-03-2013) removed a gcc hack that suppressed an "might be used
uninitialized" warning issued by older versions of gcc.

However, commit 3aa99df8 ('fast-import: clarify "inline" logic in
file_change_m', 21-03-2013) addresses an (almost) identical issue
(with very similar code), but includes additional code in it's
resolution. The solution used by this commit, unlike that used by
commit cbfd5e1c, also suppresses the -Wuninitialized warning on
older versions of gcc.

In order to suppress the warning (against the 'oe' symbol) in the
note_change_n() function, we adopt the same solution used by commit
3aa99df8.

Yeah, they are essentially the same piece of code, so I don't mind this
change.  It is odd to me that gcc gets it right in one case but not the
other, but I think we are deep into the vagaries of the compiler's code
flow analysis here, and we cannot make too many assumptions.

Were you actually triggering this warning, and if so, on what version of
gcc? 

yes, with:
    gcc v3.4.4 (cygwin)
    gcc v4.1.2 (Linux)
    msvc v15.00.30729.01 (VC/C++ 2008 express edition)
no, with:
    gcc v4.4.0 (msysgit)
    clang 3.2 (Linux)
    tcc v0.9.26 (Linux)
[lcc can't compile git; I forget why exactly.]

Thanks. I do not mind this fix, as it matches the similar code, and it
is fairly straightforward. But I am also happy to let people running gcc
v3.4.4 and other ancient compilers deal with not turning on -Werror. ;)

-Peff