[PATCH] submodule.c: Squelch a "use before assignment" warning

[PATCH] submodule.c: Squelch a "use before assignment" warning

[David Aguilar]

i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) compiler
(and probably others) mistakenly thinks variable 'right' is used
before assigned.  Work it around by giving it a fake initialization.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 submodule.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/submodule.c b/submodule.c
index 461faf0..0145a62 100644
--- a/submodule.c
+++ b/submodule.c
@@ -38,7 +38,7 @@ void show_submodule_summary(FILE *f, const char *path,
 		const char *del, const char *add, const char *reset)
 {
 	struct rev_info rev;
-	struct commit *commit, *left = left, *right;
+	struct commit *commit, *left = left, *right = NULL;
 	struct commit_list *merge_bases, *list;
 	const char *message = NULL;
 	struct strbuf sb = STRBUF_INIT;
-- 
1.6.5.3.171.ge36e

Re: [PATCH] submodule.c: Squelch a "use before assignment" warning

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) compiler
> (and probably others) mistakenly thinks variable 'right' is used
> before assigned.  Work it around by giving it a fake initialization.

We see the same "fake initialization" of 'left' on the same line.  By
initializing it to NULL, you are hinting that initializing 'right' to
NULL actually means something.

> diff --git a/submodule.c b/submodule.c
> index 461faf0..0145a62 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -38,7 +38,7 @@ void show_submodule_summary(FILE *f, const char *path,
>  		const char *del, const char *add, const char *reset)
>  {
>  	struct rev_info rev;
> -	struct commit *commit, *left = left, *right;
> +	struct commit *commit, *left = left, *right = NULL;

[PATCH] submodule.c: Squelch a "use before assignment" warning

[David Aguilar]

i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) compiler
(and probably others) mistakenly thinks variable 'right' is used
before assigned.  Work around it by giving it a fake initialization.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 submodule.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/submodule.c b/submodule.c
index 461faf0..86aad65 100644
--- a/submodule.c
+++ b/submodule.c
@@ -38,7 +38,7 @@ void show_submodule_summary(FILE *f, const char *path,
 		const char *del, const char *add, const char *reset)
 {
 	struct rev_info rev;
-	struct commit *commit, *left = left, *right;
+	struct commit *commit, *left = left, *right = right;
 	struct commit_list *merge_bases, *list;
 	const char *message = NULL;
 	struct strbuf sb = STRBUF_INIT;
-- 
1.6.5.3.171.ge36e

Re: [PATCH] submodule.c: Squelch a "use before assignment" warning

[Christoph Bartoschek]

Junio C Hamano wrote:

> David Aguilar <davvid@gmail.com> writes:
> 
>> i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493) compiler
>> (and probably others) mistakenly thinks variable 'right' is used
>> before assigned.  Work it around by giving it a fake initialization.
> 
> We see the same "fake initialization" of 'left' on the same line.  By
> initializing it to NULL, you are hinting that initializing 'right' to
> NULL actually means something.

Why is the compiler not complaining about the fake initalization? For 
initialization a value is used that is not initialized.

At least a static analyzer complains: 

"submodule.c", line 41: The variable `left' is used before its 
initialization 

Christoph

Re: [PATCH] submodule.c: Squelch a "use before assignment" warning

[Junio C Hamano]

Christoph Bartoschek <bartoschek@gmx.de> writes:

> Why is the compiler not complaining about the fake initalization? For 
> initialization a value is used that is not initialized.

That is a fairly well established idiom to tell gcc "you may mistakenly
think it isn't, but this is used".

Re: [PATCH] submodule.c: Squelch a "use before assignment" warning

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Christoph Bartoschek <bartoschek@gmx.de> writes:
>
>> Why is the compiler not complaining about the fake initalization? For 
>> initialization a value is used that is not initialized.
>
> That is a fairly well established idiom to tell gcc "you may mistakenly
> think it isn't, but this is used".

Sorry, but I was called away from the keyboard in mid sentence.  It should
have read "...this is used after getting assigned, so do not worry".

But I need to clarify a few things about this issue.

As the maintainer, I do not like a fake initialisation myself very much.
It is a declaration that "left" is always assigned before getting used by
the person who wrote _that particular version_, but later updates to the
code might introduce a codepath to incorrectly use "left" before getting
assinged to, and the fake initialisation will prevent the compiler from
catching it.

When the variable in question is a pointer, assigning NULL instead of
"left = left" will at least give us a predictable and more reproducible
breakage when later updates break the code.  I wouldn't have minded if
David's patch were to update both the existing fake initialisation and the
new one to assign NULL.  At least dereferencing such a pointer will give
us a segfault reliably.  But unfortunately there is no such "magic" values
for variables of other types (e.g. int) to help us catch uninitialized use
of variables at runtime.

By the way, if a static analyser is meant to be useful for real-world
programs, as opposed to merely an academic exercise, it should know this
convention; like it or not, it is used fairly widely.  That is, it should
check "left" is assigned before used in the rest of the function without
this "gcc hack" initializer, and if the only questionable use of "left" is
the RHS of this fake initialisation, should refrain from warning.