config.c: Make git_config() work correctly when called recursively

On Cygwin, this fixes a test failure in t3301-notes.sh (test 98, "git notes copy --for-rewrite (disabled)"). The test failure is caused by a recursive call to git_config() which has the effect of skipping to the end-of-file while processing the "notes.rewriteref" config variable. Thus, any config variables that appear after "notes.rewriteref" are simply ignored by git_config(). Also, we note that the original FILE handle is leaked as a result of the recursive call. The recursive call to git_config() is due to the "schizophrenic stat" functions on cygwin, where one of two different implementations of the l/stat functions is selected lazily, depending on some config variables. In this case, the init_copy_notes_for_rewrite() function calls git_config() with the notes_rewrite_config() callback function. This callback, while processing the "notes.rewriteref" variable, in turn calls string_list_add_refs_by_glob() to process the associated ref value. This eventually leads to a call to the get_ref_dir() function, which in turn calls stat(). On cygwin, the stat() macro leads to an indirect call to cygwin_stat_stub() which, via init_stat(), then calls git_config() in order to determine which l/stat implementation to bind to. In order to solve this problem, we modify git_config() so that the global state variables used by the config reading code is packaged up and managed on a local state stack. Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ramsay Jones committed Jun 16, 2011 at 21:24 UTC 924aaf3ef764a5e8e976f68e024ecacf54ff6306
1 file changed +50 -30
config.c
+50 -30
@@ -12,10 +12,18 @@
12
13 #define MAXNAME (256)
14
15 -static FILE *config_file;
16 -static const char *config_file_name;
17 -static int config_linenr;
18 -static int config_file_eof;
15 +typedef struct config_file {
16 + struct config_file *prev;
17 + FILE *f;
18 + const char *name;
19 + int linenr;
20 + int eof;
21 + struct strbuf value;
22 + char var[MAXNAME];
23 +} config_file;
24 +
25 +static config_file *cf;
26 +
27 static int zlib_compression_seen;
28
29 const char *config_exclusive_filename = NULL;
@@ -99,7 +107,7 @@ static int get_next_char(void)
107 FILE *f;
108
109 c = '\n';
102 - if ((f = config_file) != NULL) {
110 + if (cf && ((f = cf->f) != NULL)) {
111 c = fgetc(f);
112 if (c == '\r') {
113 /* DOS like systems */
@@ -110,9 +118,9 @@ static int get_next_char(void)
118 }
119 }
120 if (c == '\n')
113 - config_linenr++;
121 + cf->linenr++;
122 if (c == EOF) {
115 - config_file_eof = 1;
123 + cf->eof = 1;
124 c = '\n';
125 }
126 }
@@ -121,21 +129,20 @@ static int get_next_char(void)
129
130 static char *parse_value(void)
131 {
124 - static struct strbuf value = STRBUF_INIT;
132 int quote = 0, comment = 0, space = 0;
133
127 - strbuf_reset(&value);
134 + strbuf_reset(&cf->value);
135 for (;;) {
136 int c = get_next_char();
137 if (c == '\n') {
138 if (quote)
139 return NULL;
133 - return value.buf;
140 + return cf->value.buf;
141 }
142 if (comment)
143 continue;
144 if (isspace(c) && !quote) {
138 - if (value.len)
145 + if (cf->value.len)
146 space++;
147 continue;
148 }
@@ -146,7 +153,7 @@ static char *parse_value(void)
153 }
154 }
155 for (; space; space--)
149 - strbuf_addch(&value, ' ');
156 + strbuf_addch(&cf->value, ' ');
157 if (c == '\\') {
158 c = get_next_char();
159 switch (c) {
@@ -168,14 +175,14 @@ static char *parse_value(void)
175 default:
176 return NULL;
177 }
171 - strbuf_addch(&value, c);
178 + strbuf_addch(&cf->value, c);
179 continue;
180 }
181 if (c == '"') {
182 quote = 1-quote;
183 continue;
184 }
178 - strbuf_addch(&value, c);
185 + strbuf_addch(&cf->value, c);
186 }
187 }
188
@@ -192,7 +199,7 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
199 /* Get the full name */
200 for (;;) {
201 c = get_next_char();
195 - if (config_file_eof)
202 + if (cf->eof)
203 break;
204 if (!iskeychar(c))
205 break;
@@ -256,7 +263,7 @@ static int get_base_var(char *name)
263
264 for (;;) {
265 int c = get_next_char();
259 - if (config_file_eof)
266 + if (cf->eof)
267 return -1;
268 if (c == ']')
269 return baselen;
@@ -274,7 +281,7 @@ static int git_parse_file(config_fn_t fn, void *data)
281 {
282 int comment = 0;
283 int baselen = 0;
277 - static char var[MAXNAME];
284 + char *var = cf->var;
285
286 /* U+FEFF Byte Order Mark in UTF8 */
287 static const unsigned char *utf8_bom = (unsigned char *) "\xef\xbb\xbf";
@@ -298,7 +305,7 @@ static int git_parse_file(config_fn_t fn, void *data)
305 }
306 }
307 if (c == '\n') {
301 - if (config_file_eof)
308 + if (cf->eof)
309 return 0;
310 comment = 0;
311 continue;
@@ -323,7 +330,7 @@ static int git_parse_file(config_fn_t fn, void *data)
330 if (get_value(fn, data, var, baselen+1) < 0)
331 break;
332 }
326 - die("bad config file line %d in %s", config_linenr, config_file_name);
333 + die("bad config file line %d in %s", cf->linenr, cf->name);
334 }
335
336 static int parse_unit_factor(const char *end, unsigned long *val)
@@ -374,8 +381,8 @@ int git_parse_ulong(const char *value, unsigned long *ret)
381
382 static void die_bad_config(const char *name)
383 {
377 - if (config_file_name)
378 - die("bad config value for '%s' in %s", name, config_file_name);
384 + if (cf && cf->name)
385 + die("bad config value for '%s' in %s", name, cf->name);
386 die("bad config value for '%s'", name);
387 }
388
@@ -795,13 +802,24 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
802
803 ret = -1;
804 if (f) {
798 - config_file = f;
799 - config_file_name = filename;
800 - config_linenr = 1;
801 - config_file_eof = 0;
805 + config_file top;
806 +
807 + /* push config-file parsing state stack */
808 + top.prev = cf;
809 + top.f = f;
810 + top.name = filename;
811 + top.linenr = 1;
812 + top.eof = 0;
813 + strbuf_init(&top.value, 1024);
814 + cf = &top;
815 +
816 ret = git_parse_file(fn, data);
817 +
818 + /* pop config-file parsing state stack */
819 + strbuf_release(&top.value);
820 + cf = top.prev;
821 +
822 fclose(f);
804 - config_file_name = NULL;
823 }
824 return ret;
825 }
@@ -909,6 +927,7 @@ static int store_aux(const char *key, const char *value, void *cb)
927 {
928 const char *ep;
929 size_t section_len;
930 + FILE *f = cf->f;
931
932 switch (store.state) {
933 case KEY_SEEN:
@@ -920,7 +939,7 @@ static int store_aux(const char *key, const char *value, void *cb)
939 return 1;
940 }
941
923 - store.offset[store.seen] = ftell(config_file);
942 + store.offset[store.seen] = ftell(f);
943 store.seen++;
944 }
945 break;
@@ -947,19 +966,19 @@ static int store_aux(const char *key, const char *value, void *cb)
966 * Do not increment matches: this is no match, but we
967 * just made sure we are in the desired section.
968 */
950 - store.offset[store.seen] = ftell(config_file);
969 + store.offset[store.seen] = ftell(f);
970 /* fallthru */
971 case SECTION_END_SEEN:
972 case START:
973 if (matches(key, value)) {
955 - store.offset[store.seen] = ftell(config_file);
974 + store.offset[store.seen] = ftell(f);
975 store.state = KEY_SEEN;
976 store.seen++;
977 } else {
978 if (strrchr(key, '.') - key == store.baselen &&
979 !strncmp(key, store.key, store.baselen)) {
980 store.state = SECTION_SEEN;
962 - store.offset[store.seen] = ftell(config_file);
981 + store.offset[store.seen] = ftell(f);
982 }
983 }
984 }
@@ -1415,6 +1434,7 @@ int git_config_rename_section(const char *old_name, const char *new_name)
1434 struct lock_file *lock = xcalloc(sizeof(struct lock_file), 1);
1435 int out_fd;
1436 char buf[1024];
1437 + FILE *config_file;
1438
1439 if (config_exclusive_filename)
1440 config_filename = xstrdup(config_exclusive_filename);