[PATCH 2/3] Fix some "variable might be used uninitialized" warnings

[PATCH 2/3] Fix some "variable might be used uninitialized" warnings

[Ramsay Jones]

In particular, gcc complains as follows:

        CC tree-walk.o
    tree-walk.c: In function `traverse_trees':
    tree-walk.c:347: warning: 'e' might be used uninitialized in this \
        function

        CC builtin/revert.o
    builtin/revert.c: In function `verify_opt_mutually_compatible':
    builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
        this function

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 builtin/revert.c |    2 +-
 tree-walk.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 5f1cee8..30538a1 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -110,7 +110,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
 
 static void verify_opt_mutually_compatible(const char *me, ...)
 {
-	const char *opt1, *opt2;
+	const char *opt1, *opt2 = NULL;
 	va_list ap;
 
 	va_start(ap, me);
diff --git a/tree-walk.c b/tree-walk.c
index 808bb55..a8d8a66 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -344,7 +344,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 		unsigned long mask, dirmask;
 		const char *first = NULL;
 		int first_len = 0;
-		struct name_entry *e;
+		struct name_entry *e = NULL;
 		int len;
 
 		for (i = 0; i < n; i++) {
-- 
1.7.6

Re: [PATCH 2/3] Fix some "variable might be used uninitialized" warnings

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> In particular, gcc complains as follows:
>
>         CC tree-walk.o
>     tree-walk.c: In function `traverse_trees':
>     tree-walk.c:347: warning: 'e' might be used uninitialized in this \
>         function
>
>         CC builtin/revert.o
>     builtin/revert.c: In function `verify_opt_mutually_compatible':
>     builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
>         this function

Could you also add something to this effect to the commit log message:

	but I have verified that these are gcc being not careful
	enough and they are never used uninitialized.

If that is what you indeed have done, that is.

Thanks.

Re: [PATCH 2/3] Fix some "variable might be used uninitialized" warnings

[Ramsay Jones]

Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> In particular, gcc complains as follows:
>>
>>         CC tree-walk.o
>>     tree-walk.c: In function `traverse_trees':
>>     tree-walk.c:347: warning: 'e' might be used uninitialized in this \
>>         function
>>
>>         CC builtin/revert.o
>>     builtin/revert.c: In function `verify_opt_mutually_compatible':
>>     builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
>>         this function
> 
> Could you also add something to this effect to the commit log message:
> 
> 	but I have verified that these are gcc being not careful
> 	enough and they are never used uninitialized.

see below for the v2 patch.

> If that is what you indeed have done, that is.

Indeed. The builtin/revert.c warning is straight-forward, but the tree-walk.c
warning is somewhat less so! ;-)

Imagine traverse_trees() (tree-walk.c:324) was called with n == 0 (let's ignore
the effective calls to xmalloc(0) and xcalloc(0,..) at the start of that function).
At first blush it looked like 'e' would remain uninitialized in the call to
prune_traversal() at line 403.  Indeed it *would* be if you ever got to that line.
However, since the 'mask' variable (set at line 391) remains set to zero at line 401,
the flow of control leaves the loop before 'e' is used.

[I don't think traverse_trees() would ever be called with n == 0 anyway; the call
site in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)
which start from unpack_trees() are protected by "if (len)", where 'len' is unsigned.]

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH v2 2/3] Fix some "variable might be used uninitialized" warnings

In particular, gcc complains as follows:

        CC tree-walk.o
    tree-walk.c: In function `traverse_trees':
    tree-walk.c:347: warning: 'e' might be used uninitialized in this \
        function

        CC builtin/revert.o
    builtin/revert.c: In function `verify_opt_mutually_compatible':
    builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
        this function

However, I have verified that the analysis performed by gcc was too
conservative and that these variables are not, in fact, used while
uninitialized.

In order to suppress the warnings, we add an NULL pointer initializer
to the declarations of the 'e' and 'opt2' variables.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 builtin/revert.c |    2 +-
 tree-walk.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index ba27cf1..200149e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -110,7 +110,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
 
 static void verify_opt_mutually_compatible(const char *me, ...)
 {
-	const char *opt1, *opt2;
+	const char *opt1, *opt2 = NULL;
 	va_list ap;
 
 	va_start(ap, me);
diff --git a/tree-walk.c b/tree-walk.c
index 808bb55..a8d8a66 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -344,7 +344,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 		unsigned long mask, dirmask;
 		const char *first = NULL;
 		int first_len = 0;
-		struct name_entry *e;
+		struct name_entry *e = NULL;
 		int len;
 
 		for (i = 0; i < n; i++) {
-- 
1.7.6

Re: [PATCH 2/3] Fix some "variable might be used uninitialized" warnings

[Ramsay Jones]

Ramsay Jones wrote:
> Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>
>>> In particular, gcc complains as follows:
>>>
>>>         CC tree-walk.o
>>>     tree-walk.c: In function `traverse_trees':
>>>     tree-walk.c:347: warning: 'e' might be used uninitialized in this \
>>>         function
>>>
>>>         CC builtin/revert.o
>>>     builtin/revert.c: In function `verify_opt_mutually_compatible':
>>>     builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
>>>         this function
>> Could you also add something to this effect to the commit log message:
>>
>> 	but I have verified that these are gcc being not careful
>> 	enough and they are never used uninitialized.
> 
> see below for the v2 patch.
> 
>> If that is what you indeed have done, that is.
> 
> Indeed. The builtin/revert.c warning is straight-forward, but the tree-walk.c
> warning is somewhat less so! ;-)
> 
> Imagine traverse_trees() (tree-walk.c:324) was called with n == 0 (let's ignore
> the effective calls to xmalloc(0) and xcalloc(0,..) at the start of that function).
> At first blush it looked like 'e' would remain uninitialized in the call to
> prune_traversal() at line 403.  Indeed it *would* be if you ever got to that line.
> However, since the 'mask' variable (set at line 391) remains set to zero at line 401,
> the flow of control leaves the loop before 'e' is used.
> 
> [I don't think traverse_trees() would ever be called with n == 0 anyway; the call
> site in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)
> which start from unpack_trees() are protected by "if (len)", where 'len' is unsigned.]

When patches don't even make it to pu I just assume you hate them so much that
there is not much chance of them being applied and simply forget about them.
In this case, since compiler warnings are a bugbear of mine, I'm hoping that
you just forgot about this one ... :-D  [if not, sorry for the noise].

For your convenience, I've attached the patch below (rebased against current master).

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] Fix some "variable might be used uninitialized" warnings

In particular, gcc complains as follows:

        CC tree-walk.o
    tree-walk.c: In function `traverse_trees':
    tree-walk.c:347: warning: 'e' might be used uninitialized in this \
        function

        CC builtin/revert.o
    builtin/revert.c: In function `verify_opt_mutually_compatible':
    builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
        this function

However, I have verified that the analysis performed by gcc was too
conservative and that these variables are not, in fact, used while
uninitialized.

In order to suppress the warnings, we add an NULL pointer initializer
to the declarations of the 'e' and 'opt2' variables.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 builtin/revert.c |    2 +-
 tree-walk.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index ba27cf1..200149e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -110,7 +110,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
 
 static void verify_opt_mutually_compatible(const char *me, ...)
 {
-	const char *opt1, *opt2;
+	const char *opt1, *opt2 = NULL;
 	va_list ap;
 
 	va_start(ap, me);
diff --git a/tree-walk.c b/tree-walk.c
index 808bb55..a8d8a66 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -344,7 +344,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 		unsigned long mask, dirmask;
 		const char *first = NULL;
 		int first_len = 0;
-		struct name_entry *e;
+		struct name_entry *e = NULL;
 		int len;
 
 		for (i = 0; i < n; i++) {
-- 
1.7.7

Re: [PATCH 2/3] Fix some "variable might be used uninitialized" warnings

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

>> [I don't think traverse_trees() would ever be called with n == 0 anyway; the call
>> site in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)
>> which start from unpack_trees() are protected by "if (len)", where 'len' is unsigned.]
>
> When patches don't even make it to pu I just assume you hate them so much that
> there is not much chance of them being applied and simply forget about them.
> In this case, since compiler warnings are a bugbear of mine, I'm hoping that
> you just forgot about this one ... :-D  [if not, sorry for the noise].

Thanks for a reminder.

The reason a patch may not hit 'pu', unless I or other people whose
judgement I trust explicity say "the approach taken by the patch is
utterly wrong" is either because (1) the discussion for or against the
topic is still going strong and there is little chance of it getting
forgotten by everybody, (2) I do not see much discussion for or against
the topic, and I am indifferent, or (3) the patch was just lost in the
noise.

So a good default strategy for a series that do not hit 'pu' is to
re-post. Such a perseverance was what took format-patch to hit Linus's
tree in June-July 2005 timeframe---we wouldn't have the command today, had
I given up back then ;-).