git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Heiko Voigt <hvoigt@hvoigt.net>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Jens Lehmann <jens.lehmann@web.de>
Subject: Re: [PATCH 2/4] config: drop file pointer validity check in get_next_char()
Date: Thu, 28 Feb 2013 01:42:01 +0100	[thread overview]
Message-ID: <20130228004137.GA12948@sandbox-ub> (raw)
In-Reply-To: <20130227075257.GA4685@sandbox-ub>

On Wed, Feb 27, 2013 at 08:52:57AM +0100, Heiko Voigt wrote:
> On Tue, Feb 26, 2013 at 03:05:56PM -0500, Jeff King wrote:
> > On Tue, Feb 26, 2013 at 08:40:23PM +0100, Heiko Voigt wrote:
> > 
> > > The only location where cf is set in this file is in do_config_from().
> > > This function has only one callsite which is config_from_file(). In
> > > config_from_file() its ensured that the f member is set to non-zero.
> > 
> > Makes sense, although...
> > 
> > > -	if (cf && ((f = cf->f) != NULL)) {
> > > +	if (cf) {
> > > +		FILE *f = cf->f;
> > 
> > Couldn't we say the same thing about "cf" here (i.e., that it would
> > never be NULL)? Can we just get rid of this conditional entirely?
> 
> That might be true. I will look into it. Just wanted to get rid of an
> extra callback in my series.

I had a look and it might be true that cf will never be NULL in a code
path. Nevertheless its much harder to verify by looking at the code
since its a global variable. get_next_char() is called from all over the
place and I would have to look at all the code paths. As far as I know
static global variables are always initialized to zero so its safe to
check even if has not yet been explicitly initialized.

The statement if cf is not NULL all members will be initialized is much
simpler to verify since its just one place now and two places after this
series.

Cheers Heiko

  reply	other threads:[~2013-02-28  0:42 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-25  1:02 [RFC/WIP PATCH 0/3] fetch moved submodules on-demand Heiko Voigt
2013-02-25  1:04 ` [RFC/WIP PATCH 1/3] teach config parsing to read from strbuf Heiko Voigt
2013-02-25  5:54   ` Junio C Hamano
2013-02-25 17:29     ` Heiko Voigt
2013-02-26 19:30     ` [PATCH 0/4] allow more sources for config values Heiko Voigt
2013-02-26 19:38       ` [PATCH 1/4] config: factor out config file stack management Heiko Voigt
2013-02-26 19:54         ` Jeff King
2013-02-26 20:09           ` Heiko Voigt
2013-02-26 20:15             ` Jeff King
2013-02-26 22:10               ` Junio C Hamano
2013-02-27  7:51                 ` Heiko Voigt
2013-02-26 22:12             ` Junio C Hamano
2013-02-27  7:56               ` Heiko Voigt
2013-02-26 19:40       ` [PATCH 2/4] config: drop file pointer validity check in get_next_char() Heiko Voigt
2013-02-26 20:05         ` Jeff King
2013-02-27  7:52           ` Heiko Voigt
2013-02-28  0:42             ` Heiko Voigt [this message]
2013-02-28  0:54               ` Heiko Voigt
2013-02-26 19:42       ` [PATCH 3/4] config: make parsing stack struct independent from actual data source Heiko Voigt
2013-02-26 19:43       ` [PATCH 4/4] teach config parsing to read from strbuf Heiko Voigt
2013-03-07 18:42         ` Ramsay Jones
2013-03-10 16:39           ` Heiko Voigt
2013-02-26  4:55   ` [RFC/WIP PATCH 1/3] " Jeff King
2013-02-25  1:05 ` [RFC/WIP PATCH 2/3] implement fetching of moved submodules Heiko Voigt
2013-02-25  1:06 ` [RFC/WIP PATCH 3/3] submodule: simplify decision tree whether to or not to fetch Heiko Voigt

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=20130228004137.GA12948@sandbox-ub \
    --to=hvoigt@hvoigt.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jens.lehmann@web.de \
    --cc=peff@peff.net \
    /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).