Re: /* compiler workaround */ - what was the issue?

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 06/05/16 14:15, Philip Oakley wrote:
> From: "Duy Nguyen" <pclouds@gmail.com>
>> On Fri, May 6, 2016 at 4:41 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> "Philip Oakley" <philipoakley@iee.org> writes:
>>>
>>>>     int saved_namelen = saved_namelen; /* compiler workaround */
>>>>
>>>> Which then becomes an MSVC compile warning C4700: uninitialized local
>>>> variable.
>>>>
>>>> I'm wondering what was the compiler workaround being referred to? i.e. why
>>>> does it need that tweak? There's no mention of the reason in the commit
>>>> message.
>>>
>>> That was a fairly well-known workaround for GCC that issues a false
>>> warning that variable is used before initialized.  I thought we
>>> stopped using it several years ago in new code after doing a bulk
>>> sanitizing
>>
>> I guess that's 803a777 (cat-file: Fix an gcc -Wuninitialized warning -
>> 2013-03-26) and more commits around that time. The split-index commit
>> is in 2014. I must have missed the trend.
>>
>>> (I think the new recommended workaround was to initialise
>>> such a variable to the nil value like '0' for integers).
>>
>> Yep. First Jeff removed the " = xxx" part from "xxx = xxx" then Ramsay
>> added the " = NULL" back. So we probably just do "int saved_namelen =
>> 0;" in this case.
>> -- 
> Thanks,
> 
> I'll try and work up a patch - probably next week as I'm away for the weekend.

Yeah, I don't remember why these were left over from the previous
attempt to clean these up (maybe they conflicted with in-flight
topics?), but I have had a patch hanging around ... :-D

The patch below applies to master (I haven't checked for any more
additions).

ATB,
Ramsay Jones

-- >8 --
From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Subject: [PATCH] -Wuninitialized: remove a gcc specific workaround

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 builtin/rev-list.c | 2 +-
 fast-import.c      | 4 ++--
 merge-recursive.c  | 2 +-
 read-cache.c       | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 275da0d..deae1f3 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -377,7 +377,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		mark_edges_uninteresting(&revs, show_edge);
 
 	if (bisect_list) {
-		int reaches = reaches, all = all;
+		int reaches = 0, all = 0;
 
 		revs.commits = find_bisection(revs.commits, &reaches, &all,
 					      bisect_find_all);
diff --git a/fast-import.c b/fast-import.c
index 9fc7093..ca66d80 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2935,7 +2935,7 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])
 
 static void parse_get_mark(const char *p)
 {
-	struct object_entry *oe = oe;
+	struct object_entry *oe = NULL;
 	char output[42];
 
 	/* get-mark SP <object> LF */
@@ -2952,7 +2952,7 @@ static void parse_get_mark(const char *p)
 
 static void parse_cat_blob(const char *p)
 {
-	struct object_entry *oe = oe;
+	struct object_entry *oe = NULL;
 	unsigned char sha1[20];
 
 	/* cat-blob SP <object> LF */
diff --git a/merge-recursive.c b/merge-recursive.c
index 06d31ed..9cecc24 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1897,7 +1897,7 @@ int merge_recursive(struct merge_options *o,
 {
 	struct commit_list *iter;
 	struct commit *merged_common_ancestors;
-	struct tree *mrtree = mrtree;
+	struct tree *mrtree = NULL;
 	int clean;
 
 	if (show(o, 4)) {
diff --git a/read-cache.c b/read-cache.c
index d9fb78b..978d6b6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1870,7 +1870,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce,
 {
 	int size;
 	struct ondisk_cache_entry *ondisk;
-	int saved_namelen = saved_namelen; /* compiler workaround */
+	int saved_namelen = 0;
 	char *name;
 	int result;
 
-- 
2.8.0