Re: [PATCH 2/4] config: drop file pointer validity check in get_next_char()

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Feb 28, 2013, 00:42 UTC
Message-ID: <20130228004137.GA12948@sandbox-ub>
In-Reply-To: <20130227075257.GA4685@sandbox-ub>

On Wed, Feb 27, 2013 at 08:52:57AM +0100, Heiko Voigt wrote:

Show 18 quoted lines

> 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