RE: git on HP NonStop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Andreas Ericsson [mailto:ae@xxxxxx]
> Sent: Thursday, August 23, 2012 10:24 AM
> To: Joachim Schmitz
> Cc: 'Junio C Hamano'; 'Johannes Sixt'; 'Jan Engelhardt'; git@xxxxxxxxxxxxxxx
> Subject: Re: git on HP NonStop
> 
> On 08/22/2012 06:38 PM, Joachim Schmitz wrote:
> >
> >
> >> -----Original Message-----
> >> From: Junio C Hamano [mailto:gitster@xxxxxxxxx]
> >> Sent: Tuesday, August 21, 2012 4:06 AM
> >> To: Joachim Schmitz
> >> Cc: 'Johannes Sixt'; 'Jan Engelhardt'; git@xxxxxxxxxxxxxxx
> >> Subject: Re: git on HP NonStop
> >>
> >> "Joachim Schmitz" <jojo@xxxxxxxxxxxxxxxxxx> 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]