From: Philip Oakley Date: Fri, 06 May 2016 18:33:13 GMT Subject: Re: /* compiler workaround */ - what was the issue? Message-ID: <0727E275927743DF94CFEC8615D7E90B@PhilipOakley> In-Reply-To: <572CDCFF.9050607@ramsayjones.plus.com> From: "Ramsay Jones" > On 06/05/16 14:15, Philip Oakley wrote: >> From: "Duy Nguyen" >>> On Fri, May 6, 2016 at 4:41 AM, Junio C Hamano >>> wrote: >>>> "Philip Oakley" 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). Looks good to me. Catches those I've seen. Have a good weekend all. > > ATB, > Ramsay Jones > > -- >8 -- > From: Ramsay Jones > Subject: [PATCH] -Wuninitialized: remove a gcc specific workaround > > Signed-off-by: Ramsay Jones > --- > 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