On Thu, Feb 28, 2013 at 01:41:47AM +0100, Heiko Voigt wrote:
> 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.
To add some empirical information: I just ran the testsuite without the
conditional and it still passes. To me it only make sense to start the
parsing with cf initialized. But I am not familiar enough with the code
to judge whether it is safe to assume this.
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