{"thread":{"id":"27641","subject":"[PATCH 3/3] config.c: Make git_config() work correctly when called recursively","startedAt":"2011-06-16T20:24:51Z","lastAt":"2011-06-16T20:24:51Z","messageCount":1,"participants":["Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"170160","messageId":"4DFA6693.6070108@ramsay1.demon.co.uk","threadId":"27641","inReplyTo":null,"subject":"[PATCH 3/3] config.c: Make git_config() work correctly when called recursively","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-06-16T20:24:51Z","receivedAt":"2011-06-16T20:24:51Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nOn Cygwin, this fixes a test failure in t3301-notes.sh (test 98,\n\"git notes copy --for-rewrite (disabled)\").\n\nThe test failure is caused by a recursive call to git_config() which\nhas the effect of skipping to the end-of-file while processing the\n\"notes.rewriteref\" config variable. Thus, any config variables that\nappear after \"notes.rewriteref\" are simply ignored by git_config().\nAlso, we note that the original FILE handle is leaked as a result\nof the recursive call.\n\nThe recursive call to git_config() is due to the \"schizophrenic stat\"\nfunctions on cygwin, where one of two different implementations of\nthe l/stat functions is selected lazily, depending on some config\nvariables.\n\nIn this case, the init_copy_notes_for_rewrite() function calls\ngit_config() with the notes_rewrite_config() callback function.\nThis callback, while processing the \"notes.rewriteref\" variable,\nin turn calls string_list_add_refs_by_glob() to process the\nassociated ref value. This eventually leads to a call to the\nget_ref_dir() function, which in turn calls stat(). On cygwin,\nthe stat() macro leads to an indirect call to cygwin_stat_stub()\nwhich, via init_stat(), then calls git_config() in order to\ndetermine which l/stat implementation to bind to.\n\nIn order to solve this problem, we modify git_config() so that the\nglobal state variables used by the config reading code is packaged\nup and managed on a local state stack.\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n config.c |   80 ++++++++++++++++++++++++++++++++++++++-----------------------\n 1 files changed, 50 insertions(+), 30 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex e0b3b80..1fc063b 100644\n--- a/config.c\n+++ b/config.c\n@@ -12,10 +12,18 @@\n \n #define MAXNAME (256)\n \n-static FILE *config_file;\n-static const char *config_file_name;\n-static int config_linenr;\n-static int config_file_eof;\n+typedef struct config_file {\n+\tstruct config_file *prev;\n+\tFILE *f;\n+\tconst char *name;\n+\tint linenr;\n+\tint eof;\n+\tstruct strbuf value;\n+\tchar var[MAXNAME];\n+} config_file;\n+\n+static config_file *cf;\n+\n static int zlib_compression_seen;\n \n const char *config_exclusive_filename = NULL;\n@@ -99,7 +107,7 @@ static int get_next_char(void)\n \tFILE *f;\n \n \tc = '\\n';\n-\tif ((f = config_file) != NULL) {\n+\tif (cf && ((f = cf->f) != NULL)) {\n \t\tc = fgetc(f);\n \t\tif (c == '\\r') {\n \t\t\t/* DOS like systems */\n@@ -110,9 +118,9 @@ static int get_next_char(void)\n \t\t\t}\n \t\t}\n \t\tif (c == '\\n')\n-\t\t\tconfig_linenr++;\n+\t\t\tcf->linenr++;\n \t\tif (c == EOF) {\n-\t\t\tconfig_file_eof = 1;\n+\t\t\tcf->eof = 1;\n \t\t\tc = '\\n';\n \t\t}\n \t}\n@@ -121,21 +129,20 @@ static int get_next_char(void)\n \n static char *parse_value(void)\n {\n-\tstatic struct strbuf value = STRBUF_INIT;\n \tint quote = 0, comment = 0, space = 0;\n \n-\tstrbuf_reset(&value);\n+\tstrbuf_reset(&cf->value);\n \tfor (;;) {\n \t\tint c = get_next_char();\n \t\tif (c == '\\n') {\n \t\t\tif (quote)\n \t\t\t\treturn NULL;\n-\t\t\treturn value.buf;\n+\t\t\treturn cf->value.buf;\n \t\t}\n \t\tif (comment)\n \t\t\tcontinue;\n \t\tif (isspace(c) && !quote) {\n-\t\t\tif (value.len)\n+\t\t\tif (cf->value.len)\n \t\t\t\tspace++;\n \t\t\tcontinue;\n \t\t}\n@@ -146,7 +153,7 @@ static char *parse_value(void)\n \t\t\t}\n \t\t}\n \t\tfor (; space; space--)\n-\t\t\tstrbuf_addch(&value, ' ');\n+\t\t\tstrbuf_addch(&cf->value, ' ');\n \t\tif (c == '\\\\') {\n \t\t\tc = get_next_char();\n \t\t\tswitch (c) {\n@@ -168,14 +175,14 @@ static char *parse_value(void)\n \t\t\tdefault:\n \t\t\t\treturn NULL;\n \t\t\t}\n-\t\t\tstrbuf_addch(&value, c);\n+\t\t\tstrbuf_addch(&cf->value, c);\n \t\t\tcontinue;\n \t\t}\n \t\tif (c == '\"') {\n \t\t\tquote = 1-quote;\n \t\t\tcontinue;\n \t\t}\n-\t\tstrbuf_addch(&value, c);\n+\t\tstrbuf_addch(&cf->value, c);\n \t}\n }\n \n@@ -192,7 +199,7 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)\n \t/* Get the full name */\n \tfor (;;) {\n \t\tc = get_next_char();\n-\t\tif (config_file_eof)\n+\t\tif (cf->eof)\n \t\t\tbreak;\n \t\tif (!iskeychar(c))\n \t\t\tbreak;\n@@ -256,7 +263,7 @@ static int get_base_var(char *name)\n \n \tfor (;;) {\n \t\tint c = get_next_char();\n-\t\tif (config_file_eof)\n+\t\tif (cf->eof)\n \t\t\treturn -1;\n \t\tif (c == ']')\n \t\t\treturn baselen;\n@@ -274,7 +281,7 @@ static int git_parse_file(config_fn_t fn, void *data)\n {\n \tint comment = 0;\n \tint baselen = 0;\n-\tstatic char var[MAXNAME];\n+\tchar *var = cf->var;\n \n \t/* U+FEFF Byte Order Mark in UTF8 */\n \tstatic const unsigned char *utf8_bom = (unsigned char *) \"\\xef\\xbb\\xbf\";\n@@ -298,7 +305,7 @@ static int git_parse_file(config_fn_t fn, void *data)\n \t\t\t}\n \t\t}\n \t\tif (c == '\\n') {\n-\t\t\tif (config_file_eof)\n+\t\t\tif (cf->eof)\n \t\t\t\treturn 0;\n \t\t\tcomment = 0;\n \t\t\tcontinue;\n@@ -323,7 +330,7 @@ static int git_parse_file(config_fn_t fn, void *data)\n \t\tif (get_value(fn, data, var, baselen+1) < 0)\n \t\t\tbreak;\n \t}\n-\tdie(\"bad config file line %d in %s\", config_linenr, config_file_name);\n+\tdie(\"bad config file line %d in %s\", cf->linenr, cf->name);\n }\n \n static int parse_unit_factor(const char *end, unsigned long *val)\n@@ -374,8 +381,8 @@ int git_parse_ulong(const char *value, unsigned long *ret)\n \n static void die_bad_config(const char *name)\n {\n-\tif (config_file_name)\n-\t\tdie(\"bad config value for '%s' in %s\", name, config_file_name);\n+\tif (cf && cf->name)\n+\t\tdie(\"bad config value for '%s' in %s\", name, cf->name);\n \tdie(\"bad config value for '%s'\", name);\n }\n \n@@ -795,13 +802,24 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n \n \tret = -1;\n \tif (f) {\n-\t\tconfig_file = f;\n-\t\tconfig_file_name = filename;\n-\t\tconfig_linenr = 1;\n-\t\tconfig_file_eof = 0;\n+\t\tconfig_file top;\n+\n+\t\t/* push config-file parsing state stack */\n+\t\ttop.prev = cf;\n+\t\ttop.f = f;\n+\t\ttop.name = filename;\n+\t\ttop.linenr = 1;\n+\t\ttop.eof = 0;\n+\t\tstrbuf_init(&top.value, 1024);\n+\t\tcf = &top;\n+\n \t\tret = git_parse_file(fn, data);\n+\n+\t\t/* pop config-file parsing state stack */\n+\t\tstrbuf_release(&top.value);\n+\t\tcf = top.prev;\n+\n \t\tfclose(f);\n-\t\tconfig_file_name = NULL;\n \t}\n \treturn ret;\n }\n@@ -909,6 +927,7 @@ static int store_aux(const char *key, const char *value, void *cb)\n {\n \tconst char *ep;\n \tsize_t section_len;\n+\tFILE *f = cf->f;\n \n \tswitch (store.state) {\n \tcase KEY_SEEN:\n@@ -920,7 +939,7 @@ static int store_aux(const char *key, const char *value, void *cb)\n \t\t\t\treturn 1;\n \t\t\t}\n \n-\t\t\tstore.offset[store.seen] = ftell(config_file);\n+\t\t\tstore.offset[store.seen] = ftell(f);\n \t\t\tstore.seen++;\n \t\t}\n \t\tbreak;\n@@ -947,19 +966,19 @@ static int store_aux(const char *key, const char *value, void *cb)\n \t\t * Do not increment matches: this is no match, but we\n \t\t * just made sure we are in the desired section.\n \t\t */\n-\t\tstore.offset[store.seen] = ftell(config_file);\n+\t\tstore.offset[store.seen] = ftell(f);\n \t\t/* fallthru */\n \tcase SECTION_END_SEEN:\n \tcase START:\n \t\tif (matches(key, value)) {\n-\t\t\tstore.offset[store.seen] = ftell(config_file);\n+\t\t\tstore.offset[store.seen] = ftell(f);\n \t\t\tstore.state = KEY_SEEN;\n \t\t\tstore.seen++;\n \t\t} else {\n \t\t\tif (strrchr(key, '.') - key == store.baselen &&\n \t\t\t      !strncmp(key, store.key, store.baselen)) {\n \t\t\t\t\tstore.state = SECTION_SEEN;\n-\t\t\t\t\tstore.offset[store.seen] = ftell(config_file);\n+\t\t\t\t\tstore.offset[store.seen] = ftell(f);\n \t\t\t}\n \t\t}\n \t}\n@@ -1415,6 +1434,7 @@ int git_config_rename_section(const char *old_name, const char *new_name)\n \tstruct lock_file *lock = xcalloc(sizeof(struct lock_file), 1);\n \tint out_fd;\n \tchar buf[1024];\n+\tFILE *config_file;\n \n \tif (config_exclusive_filename)\n \t\tconfig_filename = xstrdup(config_exclusive_filename);\n-- \n1.7.5\n"}]}