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

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

 



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
--
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]