On Tue, Mar 12, 2013 at 05:00:56PM +0100, Heiko Voigt wrote:
> On Tue, Mar 12, 2013 at 07:00:03AM -0400, Jeff King wrote:
> > On Sun, Mar 10, 2013 at 05:58:57PM +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.
> > > [...]
> > > - 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.
>
> Ok if you say so I will do that :-). I was thinking about adding a patch
> that would remove cf as a global variable and explicitely pass it down
> to get_next_char. That makes it more obvious that it actually is != NULL.
> Looking at your callgraph I do not think its that much work. What do you
> think?
I just had a look and unfortunately there are other functions that use
this variable (namely handle_path_include) for which its not that easy
to pass this in. So I will just drop the check here.
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