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

From: Jeff King <peff@peff.net>
Date: Mar 12, 2013, 11:00 UTC
Message-ID: <20130312110003.GD11340@sigill.intra.peff.net>
In-Reply-To: <20130310165857.GC1136@sandbox-ub.fritz.box>

On Sun, Mar 10, 2013 at 05:58:57PM +0100, Heiko Voigt wrote:

Show 6 quoted lines

> 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.
> [...]
> -	if (cf && ((f = cf->f) != NULL)) {
> +	if (cf) {

I still think we can drop this conditional entirely. The complete call graph looks like:

  git_config_from_file
    -> git_parse_file
      -> get_next_char
      -> get_value
          -> get_next_char
          -> parse_value
              -> get_next_char
      -> get_base_var
          -> get_next_char
          -> get_extended_base_var
              -> get_next_char

That is, every path to get_next_char happens while we are in git_config_from_file, and that function guarantees that cf = &top, and that top.f != NULL. We do not have to even do any analysis of the conditions for each call, because we never change "cf" nor "top.f" except when we set them in git_config_from_file.

-Peff