RE: git on HP NonStop

From: Andreas Ericsson [mailto:ae@op5.se]
Sent: Thursday, August 23, 2012 10:24 AM
To: Joachim Schmitz
Cc: 'Junio C Hamano'; 'Johannes Sixt'; 'Jan Engelhardt'; git@vger.kernel.org
Subject: Re: git on HP NonStop

On 08/22/2012 06:38 PM, Joachim Schmitz wrote:

quoted

quoted

-----Original Message-----
From: Junio C Hamano [mailto:gitster@pobox.com]
Sent: Tuesday, August 21, 2012 4:06 AM
To: Joachim Schmitz
Cc: 'Johannes Sixt'; 'Jan Engelhardt'; git@vger.kernel.org
Subject: Re: git on HP NonStop

"Joachim Schmitz" [off-list ref] writes:

quoted

OK, so let's have a look at code, current git, builtin/cat-file.c,
line 196:
         void *contents = contents;

This variable is set later in an if branch (if (print_contents ==
BATCH), but not in the else branch. It is later used always under the
same condition as the one under which it is set.
Apparently is is malloc_d storage (there a "free(content);"), so
there's no harm al all in initializing it with NULL, even if it only
appeases a stupid compiler.

It actually is harmful.  See below.

Harmful to initialize with NULL or to use that undefined behavoir?

I checked what our compiler does here: after having warned about "vlues us
used before it is set: it actually dies seem to have initializes the value
to 0 resp. NULL.
So here there's no harm done in avoiding undefined behavior and set it to 0
resp NULL in the first place.

There is harm in tricking future programmers into thinking that the
initialization actually means something, which some of them do.

Hmm, OK, I can agree to that.

It's unlikely that you're the one to maintain that code forever, 

It is unlike for me to ever have to maintain this code.
Currently that's Junio's job and I won't apply for in ;-)

and the "var = var" idiom is used widely within git 

This is overstating it a bit. I went thru the entire code and reported all places I could find in an earlier email.
I went back and counted: It is used in 11 files, at 15 places, for 21 variables. 
OK, I may have missed  a few more that were in code paths my compiler didn't see, but still some 21+ isn't really much.

with a clear meaning

Only if you call undefined behavior a  'clear meaning"!

as a hint to programmers who read a bit of git code. If they aren't
used to that idiom, they usually investigate it in the code and
pretty quickly realize that what it means.

Whether I realize what it means, is irrelevant, my compiler does not and warns about it, and as per the ANSI/ICO C standard it
invokes undefined behavior.

If a proper initialization is meaningless for these cases, don't do them at all, let the stupid compiler complain about it and the
clever programmer check whether the warning is useful, but don't avoid a compiler warning on one compiler by introducing undefined
behavior and provoke a compiler warning on another.

Bye, Jojo