From: "Joachim Schmitz" <jojo@schmitz-digital.de>
To: "'Andreas Ericsson'" <ae@op5.se>
Cc: "'Junio C Hamano'" <gitster@pobox.com>,
"'Johannes Sixt'" <j.sixt@viscovery.net>,
"'Jan Engelhardt'" <jengelh@inai.de>, <git@vger.kernel.org>
Subject: RE: git on HP NonStop
Date: Thu, 23 Aug 2012 11:23:02 +0200 [thread overview]
Message-ID: <000001cd8110$e5954fc0$b0bfef40$@schmitz-digital.de> (raw)
In-Reply-To: <5035E887.3030209@op5.se>
> 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
prev parent reply other threads:[~2012-08-23 9:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <001101cd79f2$f21b3bd0$d651b370$@schmitz-digital.de>
[not found] ` <7vr4r98rfd.fsf@alter.siamese.dyndns.org>
2012-08-14 15:52 ` git on HP NonStop Joachim Schmitz
2012-08-19 16:25 ` Jan Engelhardt
2012-08-20 10:36 ` Joachim Schmitz
2012-08-20 10:56 ` Johannes Sixt
2012-08-20 11:27 ` Joachim Schmitz
2012-08-20 16:29 ` Junio C Hamano
2012-08-20 20:51 ` Joachim Schmitz
2012-08-21 2:06 ` Junio C Hamano
2012-08-22 16:38 ` Joachim Schmitz
2012-08-23 8:23 ` Andreas Ericsson
2012-08-23 9:23 ` Joachim Schmitz [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='000001cd8110$e5954fc0$b0bfef40$@schmitz-digital.de' \
--to=jojo@schmitz-digital.de \
--cc=ae@op5.se \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j.sixt@viscovery.net \
--cc=jengelh@inai.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).