RE: git on HP NonStop

From: Joachim Schmitz <jojo@schmitz-digital.de>
Date: Aug 23, 2012, 09:23 UTC
Message-ID: <000001cd8110$e5954fc0$b0bfef40$@schmitz-digital.de>
In-Reply-To: <5035E887.3030209@op5.se>

Show 42 quoted lines

> 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:
> >
> >
> >> -----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" <jojo@schmitz-digital.de> writes:
> >>
> >>> 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