On Tue, Mar 12, 2013 at 06:52:00AM -0400, Jeff King wrote: > On Sun, Mar 10, 2013 at 05:57:53PM +0100, Heiko Voigt wrote: > > > Because a config callback may start parsing a new file, the > > global context regarding the current config file is stored > > as a stack. Currently we only need to manage that stack from > > git_config_from_file. Let's factor it out to allow new > > sources of config data. > > > > [...] > > > > +static int do_config_from(struct config_file *top, config_fn_t fn, void *data) > > +{ > > + int ret; > > + > > + /* push config-file parsing state stack */ > > + top->prev = cf; > > + top->linenr = 1; > > + top->eof = 0; > > + strbuf_init(&top->value, 1024); > > + strbuf_init(&top->var, 1024); > > + cf = top; > > + > > + ret = git_parse_file(fn, data); > > + > > + /* pop config-file parsing state stack */ > > + strbuf_release(&top->value); > > + strbuf_release(&top->var); > > + cf = top->prev; > > + > > + return ret; > > +} > > Can we throw in a comment at the top here with the expected usage? In > particular, do_config_from is expecting the caller to have filled in > certain fields (at this point, top->f and top->name), but there is > nothing to make that clear. Of course. Will do that in the next iteration. How about I squash this in: diff --git a/config.c b/config.c index b8c8640..b7632c9 100644 --- a/config.c +++ b/config.c @@ -948,6 +954,9 @@ int git_default_config(const char *var, const char *value, v return 0; } +/* The fields data, name and the source specific callbacks of top need + * to be initialized before calling this function. + */ static int do_config_from_source(struct config_source *top, config_fn_t fn, voi { int ret; I would add that to the third patch: config: make parsing stack struct independent from actual data source because that contains the final modification to config_file/config_source. 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