threads / patch / 33287

patch, 2 partsfast-import: Fix an gcc -Wuninitialized warning

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

## tl;dr

4 messages between Mar 26, 2013 and Mar 28, 2013. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Ramsay Jones· Mar 26, 2013, 19:09 UTC · lore

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 <ramsay@ramsay1.demon.co.uk>
---
 fast-import.c | 1 +
 1 file changed, 1 insertion(+)

Show changes to fast-import.c +1 −0

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

Jeff King· Mar 26, 2013, 19:28 UTC · re: Ramsay Jones · lore

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

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

Show 14 quoted lines

> 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

Ramsay Jones· Mar 28, 2013, 18:20 UTC · re: Jeff King · lore

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

Jeff King wrote:

Show 24 quoted lines

> 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?

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

Jeff King· Mar 28, 2013, 18:53 UTC · re: Ramsay Jones · lore

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

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

Show 35 quoted lines

> Jeff King wrote:
> > 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? 
> 
> 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