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

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:

quoted

On Tue, Feb 26, 2013 at 08:40:23PM +0100, Heiko Voigt wrote:

quoted

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

quoted

-	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