From patchwork Wed Mar 1 00:38:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 13155391 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DE1AAC64EC7 for ; Wed, 1 Mar 2023 00:38:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229486AbjCAAi2 (ORCPT ); Tue, 28 Feb 2023 19:38:28 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41906 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229762AbjCAAiY (ORCPT ); Tue, 28 Feb 2023 19:38:24 -0500 Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 781E1A269 for ; Tue, 28 Feb 2023 16:38:21 -0800 (PST) Received: by mail-wr1-x429.google.com with SMTP id l25so11593545wrb.3 for ; Tue, 28 Feb 2023 16:38:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=S8VUUAGedcA5AmmbUPtxjHEnxgdnlFZgwXsaAAIZpuI=; b=X9mRiplDmtnDnOyv9iQobEbEYYqffV/5MZuwwpw28OlDVWWXRYNlBpuBcrhgI2g2YX WfcOJexJobU+O29PFcdAz24D8jDF4UOUfrzhZx/y2mFFww+2xCOiOrdJG3dTqrUWJo4m CnUonyQSKO3IulBGVDBgo1KQlBgLKUz02I9iJCv96f3IgFvzBe7nhEtzDFSqnAS2RrZM 1Zbs6hmpR6GaTgPu7byZa40mukulct1cPqaPYhhV+Bsl/PSWgeLxmBaBhvPSLacxheBi evU54jwukTzFjEJT+gpO1fE0dlpdlWRB2xAyQBuTDXwbI+MYLkZCv4xOoSkGNjhTYxgq IhxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=S8VUUAGedcA5AmmbUPtxjHEnxgdnlFZgwXsaAAIZpuI=; b=jcPS1RgVyWbtTl6XthF0dBmN+jCvTQ61DGiCrrH1N5RCG3sY44lGuj9G0FRmTuDUTl LKbcRVfT4kEbU4wTW/VUWaIUULBc3vNMJs4LPqsNS4TiYaxOEfs+APOQyY68yZrhbdpw 9aGHOK4XOoDbVhcUU3fEIXiWLUC0g+7NTv+tVPsz+2Nwe/sxRPRqUxgDWNrqujfzXkDR k/zB4dDgnpVw2X8EJesCY3joGHypLJdj9u/xzz0zEf+ZpgGBoOngvVmQPONBHjZoOH00 KZ0BsyKrerku74tVqTwDQEY+nvPbspyCTPl3zIsP+si2GVsHfRexi3HJpTjlMR9XG+X5 sqvA== X-Gm-Message-State: AO0yUKUMrkEx4XPDcqg+oaCAMeunTRg3A218/t/vR+qfsPv614uX7Zl1 lAysxid/3NcNBUSdxfO2woac+VV7a30= X-Google-Smtp-Source: AK7set/SE0VpY5t7wOssCh8INWnNbjEMugthd+rT8l2epEqy7QHOU9Qet+X6XGrQcORpg5VYMwCUzQ== X-Received: by 2002:a05:6000:1f10:b0:2c7:e48:8c87 with SMTP id bv16-20020a0560001f1000b002c70e488c87mr8546854wrb.25.1677631099349; Tue, 28 Feb 2023 16:38:19 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id v12-20020adfedcc000000b002c5a1bd527dsm11093059wro.96.2023.02.28.16.38.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Feb 2023 16:38:18 -0800 (PST) Message-Id: In-Reply-To: References: Date: Wed, 01 Mar 2023 00:38:12 +0000 Subject: [PATCH 1/6] config.c: plumb config_source through static fns Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jonathan Tan , Emily Shaffer , Jeff King , Derrick Stolee , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo This reduces the direct dependence on the "cf" static variable, which will make it easier to remove in a later commit. The plumbed arg is named "cs" to differentiate between it and the static variable. In some cases (public functions and config callback functions), there isn't an obvious way to plumb "struct config_source" through function args. As a workaround, add references to "cf" that we'll address in later commits. The remaining references to "cf" are direct assignments to "cf", which we'll also address in a later commit. Signed-off-by: Glen Choo --- config.c | 192 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 100 insertions(+), 92 deletions(-) diff --git a/config.c b/config.c index 00090a32fc3..84ae97741ac 100644 --- a/config.c +++ b/config.c @@ -156,7 +156,8 @@ static const char include_depth_advice[] = N_( "from\n" " %s\n" "This might be due to circular includes."); -static int handle_path_include(const char *path, struct config_include_data *inc) +static int handle_path_include(struct config_source *cs, const char *path, + struct config_include_data *inc) { int ret = 0; struct strbuf buf = STRBUF_INIT; @@ -177,14 +178,14 @@ static int handle_path_include(const char *path, struct config_include_data *inc if (!is_absolute_path(path)) { char *slash; - if (!cf || !cf->path) { + if (!cs || !cs->path) { ret = error(_("relative config includes must come from files")); goto cleanup; } - slash = find_last_dir_sep(cf->path); + slash = find_last_dir_sep(cs->path); if (slash) - strbuf_add(&buf, cf->path, slash - cf->path + 1); + strbuf_add(&buf, cs->path, slash - cs->path + 1); strbuf_addstr(&buf, path); path = buf.buf; } @@ -192,8 +193,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc if (!access_or_die(path, R_OK, 0)) { if (++inc->depth > MAX_INCLUDE_DEPTH) die(_(include_depth_advice), MAX_INCLUDE_DEPTH, path, - !cf ? "" : - cf->name ? cf->name : + !cs ? "" : + cs->name ? cs->name : "the command line"); ret = git_config_from_file(git_config_include, path, inc); inc->depth--; @@ -210,7 +211,8 @@ static void add_trailing_starstar_for_dir(struct strbuf *pat) strbuf_addstr(pat, "**"); } -static int prepare_include_condition_pattern(struct strbuf *pat) +static int prepare_include_condition_pattern(struct config_source *cs, + struct strbuf *pat) { struct strbuf path = STRBUF_INIT; char *expanded; @@ -226,11 +228,11 @@ static int prepare_include_condition_pattern(struct strbuf *pat) if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) { const char *slash; - if (!cf || !cf->path) + if (!cs || !cs->path) return error(_("relative config include " "conditionals must come from files")); - strbuf_realpath(&path, cf->path, 1); + strbuf_realpath(&path, cs->path, 1); slash = find_last_dir_sep(path.buf); if (!slash) BUG("how is this possible?"); @@ -245,7 +247,8 @@ static int prepare_include_condition_pattern(struct strbuf *pat) return prefix; } -static int include_by_gitdir(const struct config_options *opts, +static int include_by_gitdir(struct config_source *cs, + const struct config_options *opts, const char *cond, size_t cond_len, int icase) { struct strbuf text = STRBUF_INIT; @@ -261,7 +264,7 @@ static int include_by_gitdir(const struct config_options *opts, strbuf_realpath(&text, git_dir, 1); strbuf_add(&pattern, cond, cond_len); - prefix = prepare_include_condition_pattern(&pattern); + prefix = prepare_include_condition_pattern(cs, &pattern); again: if (prefix < 0) @@ -406,15 +409,16 @@ static int include_by_remote_url(struct config_include_data *inc, inc->remote_urls); } -static int include_condition_is_true(struct config_include_data *inc, +static int include_condition_is_true(struct config_source *cs, + struct config_include_data *inc, const char *cond, size_t cond_len) { const struct config_options *opts = inc->opts; if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len)) - return include_by_gitdir(opts, cond, cond_len, 0); + return include_by_gitdir(cs, opts, cond, cond_len, 0); else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len)) - return include_by_gitdir(opts, cond, cond_len, 1); + return include_by_gitdir(cs, opts, cond, cond_len, 1); else if (skip_prefix_mem(cond, cond_len, "onbranch:", &cond, &cond_len)) return include_by_branch(cond, cond_len); else if (skip_prefix_mem(cond, cond_len, "hasconfig:remote.*.url:", &cond, @@ -441,16 +445,16 @@ static int git_config_include(const char *var, const char *value, void *data) return ret; if (!strcmp(var, "include.path")) - ret = handle_path_include(value, inc); + ret = handle_path_include(cf, value, inc); if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) && - cond && include_condition_is_true(inc, cond, cond_len) && + cond && include_condition_is_true(cf, inc, cond, cond_len) && !strcmp(key, "path")) { config_fn_t old_fn = inc->fn; if (inc->opts->unconditional_remote_url) inc->fn = forbid_remote_url; - ret = handle_path_include(value, inc); + ret = handle_path_include(cf, value, inc); inc->fn = old_fn; } @@ -777,21 +781,21 @@ out: return ret; } -static int get_next_char(void) +static int get_next_char(struct config_source *cs) { - int c = cf->do_fgetc(cf); + int c = cs->do_fgetc(cs); if (c == '\r') { /* DOS like systems */ - c = cf->do_fgetc(cf); + c = cs->do_fgetc(cs); if (c != '\n') { if (c != EOF) - cf->do_ungetc(c, cf); + cs->do_ungetc(c, cs); c = '\r'; } } - if (c != EOF && ++cf->total_len > INT_MAX) { + if (c != EOF && ++cs->total_len > INT_MAX) { /* * This is an absurdly long config file; refuse to parse * further in order to protect downstream code from integer @@ -799,38 +803,38 @@ static int get_next_char(void) * but we can mark EOF and put trash in the return value, * which will trigger a parse error. */ - cf->eof = 1; + cs->eof = 1; return 0; } if (c == '\n') - cf->linenr++; + cs->linenr++; if (c == EOF) { - cf->eof = 1; - cf->linenr++; + cs->eof = 1; + cs->linenr++; c = '\n'; } return c; } -static char *parse_value(void) +static char *parse_value(struct config_source *cs) { int quote = 0, comment = 0, space = 0; - strbuf_reset(&cf->value); + strbuf_reset(&cs->value); for (;;) { - int c = get_next_char(); + int c = get_next_char(cs); if (c == '\n') { if (quote) { - cf->linenr--; + cs->linenr--; return NULL; } - return cf->value.buf; + return cs->value.buf; } if (comment) continue; if (isspace(c) && !quote) { - if (cf->value.len) + if (cs->value.len) space++; continue; } @@ -841,9 +845,9 @@ static char *parse_value(void) } } for (; space; space--) - strbuf_addch(&cf->value, ' '); + strbuf_addch(&cs->value, ' '); if (c == '\\') { - c = get_next_char(); + c = get_next_char(cs); switch (c) { case '\n': continue; @@ -863,18 +867,19 @@ static char *parse_value(void) default: return NULL; } - strbuf_addch(&cf->value, c); + strbuf_addch(&cs->value, c); continue; } if (c == '"') { quote = 1-quote; continue; } - strbuf_addch(&cf->value, c); + strbuf_addch(&cs->value, c); } } -static int get_value(config_fn_t fn, void *data, struct strbuf *name) +static int get_value(struct config_source *cs, config_fn_t fn, void *data, + struct strbuf *name) { int c; char *value; @@ -882,8 +887,8 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) /* Get the full name */ for (;;) { - c = get_next_char(); - if (cf->eof) + c = get_next_char(cs); + if (cs->eof) break; if (!iskeychar(c)) break; @@ -891,13 +896,13 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) } while (c == ' ' || c == '\t') - c = get_next_char(); + c = get_next_char(cs); value = NULL; if (c != '\n') { if (c != '=') return -1; - value = parse_value(); + value = parse_value(cs); if (!value) return -1; } @@ -906,20 +911,21 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) * the line we just parsed during the call to fn to get * accurate line number in error messages. */ - cf->linenr--; + cs->linenr--; ret = fn(name->buf, value, data); if (ret >= 0) - cf->linenr++; + cs->linenr++; return ret; } -static int get_extended_base_var(struct strbuf *name, int c) +static int get_extended_base_var(struct config_source *cs, struct strbuf *name, + int c) { - cf->subsection_case_sensitive = 0; + cs->subsection_case_sensitive = 0; do { if (c == '\n') goto error_incomplete_line; - c = get_next_char(); + c = get_next_char(cs); } while (isspace(c)); /* We require the format to be '[base "extension"]' */ @@ -928,13 +934,13 @@ static int get_extended_base_var(struct strbuf *name, int c) strbuf_addch(name, '.'); for (;;) { - int c = get_next_char(); + int c = get_next_char(cs); if (c == '\n') goto error_incomplete_line; if (c == '"') break; if (c == '\\') { - c = get_next_char(); + c = get_next_char(cs); if (c == '\n') goto error_incomplete_line; } @@ -942,25 +948,25 @@ static int get_extended_base_var(struct strbuf *name, int c) } /* Final ']' */ - if (get_next_char() != ']') + if (get_next_char(cs) != ']') return -1; return 0; error_incomplete_line: - cf->linenr--; + cs->linenr--; return -1; } -static int get_base_var(struct strbuf *name) +static int get_base_var(struct config_source *cs, struct strbuf *name) { - cf->subsection_case_sensitive = 1; + cs->subsection_case_sensitive = 1; for (;;) { - int c = get_next_char(); - if (cf->eof) + int c = get_next_char(cs); + if (cs->eof) return -1; if (c == ']') return 0; if (isspace(c)) - return get_extended_base_var(name, c); + return get_extended_base_var(cs, name, c); if (!iskeychar(c) && c != '.') return -1; strbuf_addch(name, tolower(c)); @@ -973,7 +979,8 @@ struct parse_event_data { const struct config_options *opts; }; -static int do_event(enum config_event_t type, struct parse_event_data *data) +static int do_event(struct config_source *cs, enum config_event_t type, + struct parse_event_data *data) { size_t offset; @@ -984,7 +991,7 @@ static int do_event(enum config_event_t type, struct parse_event_data *data) data->previous_type == type) return 0; - offset = cf->do_ftell(cf); + offset = cs->do_ftell(cs); /* * At EOF, the parser always "inserts" an extra '\n', therefore * the end offset of the event is the current file position, otherwise @@ -1004,12 +1011,12 @@ static int do_event(enum config_event_t type, struct parse_event_data *data) return 0; } -static int git_parse_source(config_fn_t fn, void *data, - const struct config_options *opts) +static int git_parse_source(struct config_source *cs, config_fn_t fn, + void *data, const struct config_options *opts) { int comment = 0; size_t baselen = 0; - struct strbuf *var = &cf->var; + struct strbuf *var = &cs->var; int error_return = 0; char *error_msg = NULL; @@ -1024,7 +1031,7 @@ static int git_parse_source(config_fn_t fn, void *data, for (;;) { int c; - c = get_next_char(); + c = get_next_char(cs); if (bomptr && *bomptr) { /* We are at the file beginning; skip UTF8-encoded BOM * if present. Sane editors won't put this in on their @@ -1041,12 +1048,12 @@ static int git_parse_source(config_fn_t fn, void *data, } } if (c == '\n') { - if (cf->eof) { - if (do_event(CONFIG_EVENT_EOF, &event_data) < 0) + if (cs->eof) { + if (do_event(cs, CONFIG_EVENT_EOF, &event_data) < 0) return -1; return 0; } - if (do_event(CONFIG_EVENT_WHITESPACE, &event_data) < 0) + if (do_event(cs, CONFIG_EVENT_WHITESPACE, &event_data) < 0) return -1; comment = 0; continue; @@ -1054,23 +1061,23 @@ static int git_parse_source(config_fn_t fn, void *data, if (comment) continue; if (isspace(c)) { - if (do_event(CONFIG_EVENT_WHITESPACE, &event_data) < 0) + if (do_event(cs, CONFIG_EVENT_WHITESPACE, &event_data) < 0) return -1; continue; } if (c == '#' || c == ';') { - if (do_event(CONFIG_EVENT_COMMENT, &event_data) < 0) + if (do_event(cs, CONFIG_EVENT_COMMENT, &event_data) < 0) return -1; comment = 1; continue; } if (c == '[') { - if (do_event(CONFIG_EVENT_SECTION, &event_data) < 0) + if (do_event(cs, CONFIG_EVENT_SECTION, &event_data) < 0) return -1; /* Reset prior to determining a new stem */ strbuf_reset(var); - if (get_base_var(var) < 0 || var->len < 1) + if (get_base_var(cs, var) < 0 || var->len < 1) break; strbuf_addch(var, '.'); baselen = var->len; @@ -1079,7 +1086,7 @@ static int git_parse_source(config_fn_t fn, void *data, if (!isalpha(c)) break; - if (do_event(CONFIG_EVENT_ENTRY, &event_data) < 0) + if (do_event(cs, CONFIG_EVENT_ENTRY, &event_data) < 0) return -1; /* @@ -1089,42 +1096,42 @@ static int git_parse_source(config_fn_t fn, void *data, */ strbuf_setlen(var, baselen); strbuf_addch(var, tolower(c)); - if (get_value(fn, data, var) < 0) + if (get_value(cs, fn, data, var) < 0) break; } - if (do_event(CONFIG_EVENT_ERROR, &event_data) < 0) + if (do_event(cs, CONFIG_EVENT_ERROR, &event_data) < 0) return -1; - switch (cf->origin_type) { + switch (cs->origin_type) { case CONFIG_ORIGIN_BLOB: error_msg = xstrfmt(_("bad config line %d in blob %s"), - cf->linenr, cf->name); + cs->linenr, cs->name); break; case CONFIG_ORIGIN_FILE: error_msg = xstrfmt(_("bad config line %d in file %s"), - cf->linenr, cf->name); + cs->linenr, cs->name); break; case CONFIG_ORIGIN_STDIN: error_msg = xstrfmt(_("bad config line %d in standard input"), - cf->linenr); + cs->linenr); break; case CONFIG_ORIGIN_SUBMODULE_BLOB: error_msg = xstrfmt(_("bad config line %d in submodule-blob %s"), - cf->linenr, cf->name); + cs->linenr, cs->name); break; case CONFIG_ORIGIN_CMDLINE: error_msg = xstrfmt(_("bad config line %d in command line %s"), - cf->linenr, cf->name); + cs->linenr, cs->name); break; default: error_msg = xstrfmt(_("bad config line %d in %s"), - cf->linenr, cf->name); + cs->linenr, cs->name); } switch (opts && opts->error_action ? opts->error_action : - cf->default_error_action) { + cs->default_error_action) { case CONFIG_ERROR_DIE: die("%s", error_msg); break; @@ -1266,7 +1273,8 @@ int git_parse_ssize_t(const char *value, ssize_t *ret) } NORETURN -static void die_bad_number(const char *name, const char *value) +static void die_bad_number(struct config_source *cs, const char *name, + const char *value) { const char *error_type = (errno == ERANGE) ? N_("out of range") : N_("invalid unit"); @@ -1275,28 +1283,28 @@ static void die_bad_number(const char *name, const char *value) if (!value) value = ""; - if (!(cf && cf->name)) + if (!(cs && cs->name)) die(_(bad_numeric), value, name, _(error_type)); - switch (cf->origin_type) { + switch (cs->origin_type) { case CONFIG_ORIGIN_BLOB: die(_("bad numeric config value '%s' for '%s' in blob %s: %s"), - value, name, cf->name, _(error_type)); + value, name, cs->name, _(error_type)); case CONFIG_ORIGIN_FILE: die(_("bad numeric config value '%s' for '%s' in file %s: %s"), - value, name, cf->name, _(error_type)); + value, name, cs->name, _(error_type)); case CONFIG_ORIGIN_STDIN: die(_("bad numeric config value '%s' for '%s' in standard input: %s"), value, name, _(error_type)); case CONFIG_ORIGIN_SUBMODULE_BLOB: die(_("bad numeric config value '%s' for '%s' in submodule-blob %s: %s"), - value, name, cf->name, _(error_type)); + value, name, cs->name, _(error_type)); case CONFIG_ORIGIN_CMDLINE: die(_("bad numeric config value '%s' for '%s' in command line %s: %s"), - value, name, cf->name, _(error_type)); + value, name, cs->name, _(error_type)); default: die(_("bad numeric config value '%s' for '%s' in %s: %s"), - value, name, cf->name, _(error_type)); + value, name, cs->name, _(error_type)); } } @@ -1304,7 +1312,7 @@ int git_config_int(const char *name, const char *value) { int ret; if (!git_parse_int(value, &ret)) - die_bad_number(name, value); + die_bad_number(cf, name, value); return ret; } @@ -1312,7 +1320,7 @@ int64_t git_config_int64(const char *name, const char *value) { int64_t ret; if (!git_parse_int64(value, &ret)) - die_bad_number(name, value); + die_bad_number(cf, name, value); return ret; } @@ -1320,7 +1328,7 @@ unsigned long git_config_ulong(const char *name, const char *value) { unsigned long ret; if (!git_parse_ulong(value, &ret)) - die_bad_number(name, value); + die_bad_number(cf, name, value); return ret; } @@ -1328,7 +1336,7 @@ ssize_t git_config_ssize_t(const char *name, const char *value) { ssize_t ret; if (!git_parse_ssize_t(value, &ret)) - die_bad_number(name, value); + die_bad_number(cf, name, value); return ret; } @@ -1948,7 +1956,7 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data, strbuf_init(&top->var, 1024); cf = top; - ret = git_parse_source(fn, data, opts); + ret = git_parse_source(cf, fn, data, opts); /* pop config-file parsing state stack */ strbuf_release(&top->value); From patchwork Wed Mar 1 00:38:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 13155390 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B5868C64EC4 for ; Wed, 1 Mar 2023 00:38:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229775AbjCAAiZ (ORCPT ); Tue, 28 Feb 2023 19:38:25 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41890 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229582AbjCAAiX (ORCPT ); Tue, 28 Feb 2023 19:38:23 -0500 Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 15A28B765 for ; Tue, 28 Feb 2023 16:38:22 -0800 (PST) Received: by mail-wm1-x331.google.com with SMTP id l7-20020a05600c4f0700b003e79fa98ce1so6919018wmq.2 for ; Tue, 28 Feb 2023 16:38:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=J/IwQTJmndKxRu4J2lk4OcPkcSdZ0ZcfzN15S5g41xg=; b=VKeciw4AlKmWv1pMLf0Wx2DG826vfaz8+eiToblE9Zcmmpfda6CbCrsOOI926hzWB9 L1JCTsQu44Y2CF7wPdf45J5tPPfwzDsLuSNGfhX5dZWbocXkHOigqvRTTZIcNM7M7a/B ONyBNkt3XpVAbn9MgsYzHvBlhfj3f02XgMAsUFuqN63ARIHoP3QJiAcra7iSa4zMkfnq ATJQx/gAvR9Zwb2Sxp44GAglG1kbTSQEtbr/LZy5irMzD8U3zS7dYpIXBax90zAfh9ze y+S2xRbCFk9Pd509N5H/FGH3e71Fo/rZdoXKjNWN7TYckZLZjn3yqnq/9SAxxQKVjIdd xhgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=J/IwQTJmndKxRu4J2lk4OcPkcSdZ0ZcfzN15S5g41xg=; b=OMokRl0MMmDKwvQc5vYUHH5ldgC+RM9YpcUiH0UI6qZopGq0ld0rRwjV8tUFYnN6T6 8EozHbromut305nqC5ONukqmc0qvnyHlRODqkuMBLglnzAO91wF2Kqh/q5g6BKHjgEmV vhJxKovj3DhAICDfy7LegS+HTfsyfMdH6RJMXHPJZQX6GlA7tzIKe4uo5mUfVyvvJ+s4 xMuiaFY3LgNH1+VD4o36w5ye2bLqZOf6UgtV3JMqirQRMF0WkQhj4tv3mchXVDGH3N6S Y06ujUI5c6IEnZawWaMEs2+ihFZxRg3Hc7s7XPnaF+8lOuEnDBC4chGeyNFuR/2+7PeA pV5A== X-Gm-Message-State: AO0yUKVdxHRs3WFXGwMpxuSvvwdL2pBx9NzwPdac0yOPxQFRbcDAR8Xo 0tJYIf6TYGrR/TlX9fjz/KRFTE6G4vM= X-Google-Smtp-Source: AK7set/mShRpKGejCK5/zxiriKkEla2uMiuzY9mksWk+mAAucI61RvtUtcaAdnnWihPcHlGmI7l8RQ== X-Received: by 2002:a05:600c:4e12:b0:3eb:3998:8bca with SMTP id b18-20020a05600c4e1200b003eb39988bcamr8572423wmq.17.1677631100425; Tue, 28 Feb 2023 16:38:20 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id v7-20020a05600c470700b003eb3933ef10sm12492618wmo.46.2023.02.28.16.38.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Feb 2023 16:38:19 -0800 (PST) Message-Id: <9f72cbb8d78265c85c3e6405a5a61cf35db5ebca.1677631097.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 01 Mar 2023 00:38:13 +0000 Subject: [PATCH 2/6] config.c: don't assign to "cf" directly Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jonathan Tan , Emily Shaffer , Jeff King , Derrick Stolee , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo To make "cf" easier to remove, replace all direct assignments to it with function calls. This refactor has an additional maintainability benefit: all of these functions were manually implementing stack pop/push semantics on "struct config_source", so replacing them with function calls allows us to only implement this logic once. In this process, perform some now-obvious clean ups: - Drop some unnecessary "cf" assignments in populate_remote_urls(). Since it was introduced in 399b198489 (config: include file if remote URL matches a glob, 2022-01-18), it has stored and restored the value of "cf" to ensure that it doesn't get accidentally mutated. However, this was never necessary since "do_config_from()" already pushes/pops "cf" further down the call chain. - Zero out every "struct config_source" with a dedicated initializer. This matters because the "struct config_source" is assigned to "cf" and we later 'pop the stack' by assigning "cf = cf->prev", but "cf->prev" could be pointing to uninitialized garbage. Fortunately, this has never bothered us since we never try to read "cf" except while iterating through config, in which case, "cf" is either set to a sensible value (when parsing a file), or it is ignored (when iterating a configset). Later in the series, zero-ing out memory will also let us enforce the constraint that "cf" and "current_config_kvi" are never non-NULL together. Signed-off-by: Glen Choo --- config.c | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/config.c b/config.c index 84ae97741ac..1f89addc771 100644 --- a/config.c +++ b/config.c @@ -49,6 +49,7 @@ struct config_source { int (*do_ungetc)(int c, struct config_source *conf); long (*do_ftell)(struct config_source *c); }; +#define CONFIG_SOURCE_INIT { 0 } /* * These variables record the "current" config source, which @@ -78,6 +79,23 @@ static struct key_value_info *current_config_kvi; */ static enum config_scope current_parsing_scope; +static inline void config_state_push_source(struct config_source *top) +{ + if (cf) + top->prev = cf; + cf = top; +} + +static inline struct config_source *config_state_pop_source() +{ + struct config_source *ret; + if (!cf) + BUG("tried to pop config source, but we weren't reading config"); + ret = cf; + cf = cf->prev; + return ret; +} + static int pack_compression_seen; static int zlib_compression_seen; @@ -345,14 +363,12 @@ static void populate_remote_urls(struct config_include_data *inc) { struct config_options opts; - struct config_source *store_cf = cf; struct key_value_info *store_kvi = current_config_kvi; enum config_scope store_scope = current_parsing_scope; opts = *inc->opts; opts.unconditional_remote_url = 1; - cf = NULL; current_config_kvi = NULL; current_parsing_scope = 0; @@ -360,7 +376,6 @@ static void populate_remote_urls(struct config_include_data *inc) string_list_init_dup(inc->remote_urls); config_with_options(add_remote_url, inc->remote_urls, inc->config_source, &opts); - cf = store_cf; current_config_kvi = store_kvi; current_parsing_scope = store_scope; } @@ -714,12 +729,10 @@ int git_config_from_parameters(config_fn_t fn, void *data) struct strvec to_free = STRVEC_INIT; int ret = 0; char *envw = NULL; - struct config_source source; + struct config_source source = CONFIG_SOURCE_INIT; - memset(&source, 0, sizeof(source)); - source.prev = cf; source.origin_type = CONFIG_ORIGIN_CMDLINE; - cf = &source; + config_state_push_source(&source); env = getenv(CONFIG_COUNT_ENVIRONMENT); if (env) { @@ -777,7 +790,7 @@ out: strbuf_release(&envvar); strvec_clear(&to_free); free(envw); - cf = source.prev; + config_state_pop_source(); return ret; } @@ -1948,20 +1961,19 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data, int ret; /* push config-file parsing state stack */ - top->prev = cf; top->linenr = 1; top->eof = 0; top->total_len = 0; strbuf_init(&top->value, 1024); strbuf_init(&top->var, 1024); - cf = top; + config_state_push_source(top); - ret = git_parse_source(cf, fn, data, opts); + ret = git_parse_source(top, fn, data, opts); /* pop config-file parsing state stack */ strbuf_release(&top->value); strbuf_release(&top->var); - cf = top->prev; + config_state_pop_source(); return ret; } @@ -1971,7 +1983,7 @@ static int do_config_from_file(config_fn_t fn, const char *name, const char *path, FILE *f, void *data, const struct config_options *opts) { - struct config_source top; + struct config_source top = CONFIG_SOURCE_INIT; int ret; top.u.file = f; @@ -2023,7 +2035,7 @@ int git_config_from_mem(config_fn_t fn, const char *name, const char *buf, size_t len, void *data, const struct config_options *opts) { - struct config_source top; + struct config_source top = CONFIG_SOURCE_INIT; top.u.buf.buf = buf; top.u.buf.len = len; From patchwork Wed Mar 1 00:38:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 13155392 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 43B6BC64EC7 for ; Wed, 1 Mar 2023 00:38:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229819AbjCAAig (ORCPT ); Tue, 28 Feb 2023 19:38:36 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41952 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229769AbjCAAiY (ORCPT ); Tue, 28 Feb 2023 19:38:24 -0500 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6129EC16D for ; Tue, 28 Feb 2023 16:38:22 -0800 (PST) Received: by mail-wr1-x434.google.com with SMTP id t15so11576775wrz.7 for ; Tue, 28 Feb 2023 16:38:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=xCc9lOosAd//zL4d2u6Z9Kr06d8OAAHP2k4PrxBPljQ=; b=MImYs5W8nL+PjKaVbKGjy2OZMq/5ur9azSTPjBPfJw+dzmSlqSexFHEVchIVol33aN eg5DKosH8fycjMEa5fpKfZeGNTdcjI5rXZhCIRHxoazCw/dJvSpvgNtvPAo5YM3+IFWS ARk/ejAi1p05M7eY/lGO0mD1S5t3y/1lmk0l39yx5YqnwWPlmly/AybSOIR5owbHmVQ1 6yCMolLZRhbipTG/PFD1K57f6jMPD/cknmgNoMXUFdpQz9K7lG3uxgkXvFBeM4fKaqTQ +uU7JQ0PLFyTeIwgjOVARXaryQ+Mu9/wEtsC2LvV35ebaaQKH+GbqI/4yIRuxsmWWqaw 0wwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=xCc9lOosAd//zL4d2u6Z9Kr06d8OAAHP2k4PrxBPljQ=; b=chm/7tf4BCic5zFQ63DCdmfNUHvj0dv0PZ7QctPrY9IkHWuRuXPd2pwPvhENZSKrTr byQ16rC6sc2o6wrnev1a4pnE8rFoyq51maxveaG59kk69UHun9imYsJMOmpLalIb68+m tR1ctCe0ZGw+5G2pfXOJZUmoH0D2XF5Lrwd2pjb2CmqkueUI9CAGlwvEj++rRwFCIJZ7 e8PDZbDBHm5ehzSk/VP5LUakD8A2tsgzuUm6tMvjOW7wvRwXXZ6qtBsdjA5GcCxpt384 1HP9N4UuQlOp9IbG9i317O4b9kn+4MJWdvPgvpu31QnKFDq1VbePI47KJfAfPSJi1KVZ IuUg== X-Gm-Message-State: AO0yUKU5dOKqdqkxnoMwV6NnozcjK0xK+ETQoHWDrib0t7m2nmzEaXpj R+4GVqm0gh3iVgbOs5YJTGRw1Iyonjk= X-Google-Smtp-Source: AK7set+TBxBrj7QhuP8RKCcjfMLEfXheRAJi2++FnpjO/+SmDPYam16Ij8JPoj9gnb2Ph8uDcYLQ0g== X-Received: by 2002:a05:6000:104d:b0:2c5:899f:3412 with SMTP id c13-20020a056000104d00b002c5899f3412mr2845253wrx.26.1677631101156; Tue, 28 Feb 2023 16:38:21 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id p1-20020a056000018100b002c54fb024b2sm11214747wrx.61.2023.02.28.16.38.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Feb 2023 16:38:20 -0800 (PST) Message-Id: <751ce3e927d392530038006a3f1e3a9538ce2127.1677631097.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 01 Mar 2023 00:38:14 +0000 Subject: [PATCH 3/6] config.c: create config_reader and the_reader Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jonathan Tan , Emily Shaffer , Jeff King , Derrick Stolee , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo Create "struct config_reader" to hold the state of the config source currently being read. Then, create a static instance of it, "the_reader", and use "the_reader.source" to replace references to "cf" in public functions. This doesn't create much immediate benefit (since we're mostly replacing static variables with a bigger static variable), but it prepares us for a future where this state doesn't have to be global; the "struct config_reader" could be provided by the caller, or constructed internally by a function like "do_config_from()". A more typical approach would be to put this struct on "the_repository", but that's a worse fit for this use case since config reading is not scoped to a repository. E.g. we can read config before the repository is known ("read_very_early_config()"), blatantly ignore the repo ("read_protected_config()"), or read only from a file ("git_config_from_file()"). This is especially evident in t5318 and t9210, where test-tool and scalar parse config but don't fully initialize "the_repository". We could have also replaced the references to "cf" in callback functions (which are the only ones left), but we'll eventually plumb "the_reader" through the callback "*data" arg, which will allow us to rename "cf" to "cs" without changing line lengths. Until we remove "cf" altogether, add logic to "config_reader_*_source()" to keep "cf" and "the_reader.source" in sync. Signed-off-by: Glen Choo --- config.c | 80 +++++++++++++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/config.c b/config.c index 1f89addc771..866cd54dd40 100644 --- a/config.c +++ b/config.c @@ -51,6 +51,12 @@ struct config_source { }; #define CONFIG_SOURCE_INIT { 0 } +struct config_reader { + struct config_source *source; +}; +/* Only public functions should reference the_reader. */ +static struct config_reader the_reader; + /* * These variables record the "current" config source, which * can be accessed by parsing callbacks. @@ -66,6 +72,9 @@ struct config_source { * at the variables, it's either a bug for it to be called in the first place, * or it's a function which can be reused for non-config purposes, and should * fall back to some sane behavior). + * + * FIXME "cf" has been replaced by "the_reader.source", remove + * "cf" once we plumb "the_reader" through all of the callback functions. */ static struct config_source *cf; static struct key_value_info *current_config_kvi; @@ -79,20 +88,25 @@ static struct key_value_info *current_config_kvi; */ static enum config_scope current_parsing_scope; -static inline void config_state_push_source(struct config_source *top) +static inline void config_reader_push_source(struct config_reader *reader, + struct config_source *top) { - if (cf) - top->prev = cf; - cf = top; + if (reader->source) + top->prev = reader->source; + reader->source = top; + /* FIXME remove this when cf is removed. */ + cf = reader->source; } -static inline struct config_source *config_state_pop_source() +static inline struct config_source *config_reader_pop_source(struct config_reader *reader) { struct config_source *ret; - if (!cf) + if (!reader->source) BUG("tried to pop config source, but we weren't reading config"); - ret = cf; - cf = cf->prev; + ret = reader->source; + reader->source = reader->source->prev; + /* FIXME remove this when cf is removed. */ + cf = reader->source; return ret; } @@ -732,7 +746,7 @@ int git_config_from_parameters(config_fn_t fn, void *data) struct config_source source = CONFIG_SOURCE_INIT; source.origin_type = CONFIG_ORIGIN_CMDLINE; - config_state_push_source(&source); + config_reader_push_source(&the_reader, &source); env = getenv(CONFIG_COUNT_ENVIRONMENT); if (env) { @@ -790,7 +804,7 @@ out: strbuf_release(&envvar); strvec_clear(&to_free); free(envw); - config_state_pop_source(); + config_reader_pop_source(&the_reader); return ret; } @@ -1325,7 +1339,7 @@ int git_config_int(const char *name, const char *value) { int ret; if (!git_parse_int(value, &ret)) - die_bad_number(cf, name, value); + die_bad_number(the_reader.source, name, value); return ret; } @@ -1333,7 +1347,7 @@ int64_t git_config_int64(const char *name, const char *value) { int64_t ret; if (!git_parse_int64(value, &ret)) - die_bad_number(cf, name, value); + die_bad_number(the_reader.source, name, value); return ret; } @@ -1341,7 +1355,7 @@ unsigned long git_config_ulong(const char *name, const char *value) { unsigned long ret; if (!git_parse_ulong(value, &ret)) - die_bad_number(cf, name, value); + die_bad_number(the_reader.source, name, value); return ret; } @@ -1349,7 +1363,7 @@ ssize_t git_config_ssize_t(const char *name, const char *value) { ssize_t ret; if (!git_parse_ssize_t(value, &ret)) - die_bad_number(cf, name, value); + die_bad_number(the_reader.source, name, value); return ret; } @@ -1955,7 +1969,8 @@ int git_default_config(const char *var, const char *value, void *cb) * fgetc, ungetc, ftell of top need to be initialized before calling * this function. */ -static int do_config_from(struct config_source *top, config_fn_t fn, void *data, +static int do_config_from(struct config_reader *reader, + struct config_source *top, config_fn_t fn, void *data, const struct config_options *opts) { int ret; @@ -1966,22 +1981,23 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data, top->total_len = 0; strbuf_init(&top->value, 1024); strbuf_init(&top->var, 1024); - config_state_push_source(top); + config_reader_push_source(reader, top); ret = git_parse_source(top, fn, data, opts); /* pop config-file parsing state stack */ strbuf_release(&top->value); strbuf_release(&top->var); - config_state_pop_source(); + config_reader_pop_source(reader); return ret; } -static int do_config_from_file(config_fn_t fn, - const enum config_origin_type origin_type, - const char *name, const char *path, FILE *f, - void *data, const struct config_options *opts) +static int do_config_from_file(struct config_reader *reader, + config_fn_t fn, + const enum config_origin_type origin_type, + const char *name, const char *path, FILE *f, + void *data, const struct config_options *opts) { struct config_source top = CONFIG_SOURCE_INIT; int ret; @@ -1996,15 +2012,15 @@ static int do_config_from_file(config_fn_t fn, top.do_ftell = config_file_ftell; flockfile(f); - ret = do_config_from(&top, fn, data, opts); + ret = do_config_from(reader, &top, fn, data, opts); funlockfile(f); return ret; } static int git_config_from_stdin(config_fn_t fn, void *data) { - return do_config_from_file(fn, CONFIG_ORIGIN_STDIN, "", NULL, stdin, - data, NULL); + return do_config_from_file(&the_reader, fn, CONFIG_ORIGIN_STDIN, "", + NULL, stdin, data, NULL); } int git_config_from_file_with_options(config_fn_t fn, const char *filename, @@ -2018,8 +2034,8 @@ int git_config_from_file_with_options(config_fn_t fn, const char *filename, BUG("filename cannot be NULL"); f = fopen_or_warn(filename, "r"); if (f) { - ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename, - filename, f, data, opts); + ret = do_config_from_file(&the_reader, fn, CONFIG_ORIGIN_FILE, + filename, filename, f, data, opts); fclose(f); } return ret; @@ -2048,7 +2064,7 @@ int git_config_from_mem(config_fn_t fn, top.do_ungetc = config_buf_ungetc; top.do_ftell = config_buf_ftell; - return do_config_from(&top, fn, data, opts); + return do_config_from(&the_reader, &top, fn, data, opts); } int git_config_from_blob_oid(config_fn_t fn, @@ -3791,8 +3807,8 @@ const char *current_config_origin_type(void) int type; if (current_config_kvi) type = current_config_kvi->origin_type; - else if(cf) - type = cf->origin_type; + else if(the_reader.source) + type = the_reader.source->origin_type; else BUG("current_config_origin_type called outside config callback"); @@ -3837,8 +3853,8 @@ const char *current_config_name(void) const char *name; if (current_config_kvi) name = current_config_kvi->filename; - else if (cf) - name = cf->name; + else if (the_reader.source) + name = the_reader.source->name; else BUG("current_config_name called outside config callback"); return name ? name : ""; @@ -3857,7 +3873,7 @@ int current_config_line(void) if (current_config_kvi) return current_config_kvi->linenr; else - return cf->linenr; + return the_reader.source->linenr; } int lookup_config(const char **mapping, int nr_mapping, const char *var) From patchwork Wed Mar 1 00:38:15 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 13155394 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D099C64EC7 for ; Wed, 1 Mar 2023 00:38:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229686AbjCAAij (ORCPT ); Tue, 28 Feb 2023 19:38:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41988 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229780AbjCAAiZ (ORCPT ); Tue, 28 Feb 2023 19:38:25 -0500 Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6CB68C17D for ; Tue, 28 Feb 2023 16:38:23 -0800 (PST) Received: by mail-wr1-x435.google.com with SMTP id g3so2701223wri.6 for ; Tue, 28 Feb 2023 16:38:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=UNsLq9fgvXYD/pip2Ciao8JR7OKIvsPhPgoItWy+XIE=; b=LmXT/x/Ro+lBNmZ9ixibBsb2/FWqPV7AuFjQw+ppbeibb9bwR7YlSJPZF3IL/DdY87 QUyM1ok9wscxHWPQJKZ/1krVmnbrQExfq+2s3wA0nofH933gQozWN3/EPRRuotOwshnV rc4w0Xab+PDsTGFaXTwq7lt10pc0eXp7e4/PlzGwE7VwuZigttiYVhm4lGRJ31IAifxV kqDG7LB0vMKE8I9m2y82P5FC6Xgic7iNY35PWsBsCE9IumeBZ+6FbHd5Wr8EWBoPVTQk NsJ4Gcngv1mm3Nyullr7X8wF6O3rnGN4Sfz/Z1LozZesutB6bLwJZL5jxK/m09NxC4t/ bWnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=UNsLq9fgvXYD/pip2Ciao8JR7OKIvsPhPgoItWy+XIE=; b=lRiVSMsP5uLQcYF0LHQOjiBau3yTFGYP65TW02kNh0SYPaOq0Kqys426bPzpHdUmdU FkeGAnHjQuU4c0p/c+tEYO2iWbrX35sK8ELL3hI+et93ZYQcEWKiGzor9cWvVHBNipeZ 8wid4W3GvDZrQ2VvGdm5EbMcPBLa6hJK6H4Oh7Uuh+YELSrUJws4r9TQTByXlQ/zHqzi 50dxL0Qz3VL7Faq6zqimdjvbTGdMj0Kl1VSABg7dmNI3Qbu6xveHFKdrBepEWb1Dx7Pu nsjxP/CFvC0W05I3/Ef2a7Dj5cpJJcOb8fMUKtllgxkvC0IlhprNuhkYTwbfo1MZvQh4 sPSw== X-Gm-Message-State: AO0yUKXITwth59FMBLv0S0Kj484p26tq9GE/JCHRscGK/a0CzWBJCgTZ TB/rwi0ZskUV2pZ/NHc7tuEkI50+ndY= X-Google-Smtp-Source: AK7set8+eFv2Mav3cjYLk0O+cVSgDbSRv4DhtESEoCPKO7hMwQaYYizZahv0pEo7qCS8rUO6jab1iQ== X-Received: by 2002:adf:e505:0:b0:2c8:a6ea:c00c with SMTP id j5-20020adfe505000000b002c8a6eac00cmr3996688wrm.69.1677631101820; Tue, 28 Feb 2023 16:38:21 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id n7-20020a5d4c47000000b002c5534db60bsm10956866wrt.71.2023.02.28.16.38.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Feb 2023 16:38:21 -0800 (PST) Message-Id: <74a63fed7054da8049d4a32ecdb582726368c5a8.1677631097.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 01 Mar 2023 00:38:15 +0000 Subject: [PATCH 4/6] config.c: plumb the_reader through callbacks Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jonathan Tan , Emily Shaffer , Jeff King , Derrick Stolee , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo The remaining references to "cf" are in config callback functions. Remove them by plumbing "struct config_reader" via the "*data" arg. **RFC NOTE** If we had a way to expose "struct config_reader" to the config callback functions (the 'extra work' in the cover letter), we wouldn't need to also pass it via the "*data" arg. This is more of a hack to avoid doing that work now. Signed-off-by: Glen Choo --- config.c | 78 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/config.c b/config.c index 866cd54dd40..9676734b1b7 100644 --- a/config.c +++ b/config.c @@ -58,6 +58,9 @@ struct config_reader { static struct config_reader the_reader; /* + * FIXME The comments are temporarily out of date since "cf" been moved to + * the_reader, but not current_*. + * * These variables record the "current" config source, which * can be accessed by parsing callbacks. * @@ -72,11 +75,7 @@ static struct config_reader the_reader; * at the variables, it's either a bug for it to be called in the first place, * or it's a function which can be reused for non-config purposes, and should * fall back to some sane behavior). - * - * FIXME "cf" has been replaced by "the_reader.source", remove - * "cf" once we plumb "the_reader" through all of the callback functions. */ -static struct config_source *cf; static struct key_value_info *current_config_kvi; /* @@ -94,8 +93,6 @@ static inline void config_reader_push_source(struct config_reader *reader, if (reader->source) top->prev = reader->source; reader->source = top; - /* FIXME remove this when cf is removed. */ - cf = reader->source; } static inline struct config_source *config_reader_pop_source(struct config_reader *reader) @@ -105,8 +102,6 @@ static inline struct config_source *config_reader_pop_source(struct config_reade BUG("tried to pop config source, but we weren't reading config"); ret = reader->source; reader->source = reader->source->prev; - /* FIXME remove this when cf is removed. */ - cf = reader->source; return ret; } @@ -171,6 +166,7 @@ struct config_include_data { void *data; const struct config_options *opts; struct git_config_source *config_source; + struct config_reader *config_reader; /* * All remote URLs discovered when reading all config files. @@ -461,6 +457,7 @@ static int include_condition_is_true(struct config_source *cs, static int git_config_include(const char *var, const char *value, void *data) { struct config_include_data *inc = data; + struct config_source *cs = inc->config_reader->source; const char *cond, *key; size_t cond_len; int ret; @@ -474,16 +471,16 @@ static int git_config_include(const char *var, const char *value, void *data) return ret; if (!strcmp(var, "include.path")) - ret = handle_path_include(cf, value, inc); + ret = handle_path_include(cs, value, inc); if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) && - cond && include_condition_is_true(cf, inc, cond, cond_len) && + cond && include_condition_is_true(cs, inc, cond, cond_len) && !strcmp(key, "path")) { config_fn_t old_fn = inc->fn; if (inc->opts->unconditional_remote_url) inc->fn = forbid_remote_url; - ret = handle_path_include(cf, value, inc); + ret = handle_path_include(cs, value, inc); inc->fn = old_fn; } @@ -2224,6 +2221,7 @@ int config_with_options(config_fn_t fn, void *data, inc.data = data; inc.opts = opts; inc.config_source = config_source; + inc.config_reader = &the_reader; fn = git_config_include; data = &inc; } @@ -2344,7 +2342,9 @@ static struct config_set_element *configset_find_element(struct config_set *cs, return found_entry; } -static int configset_add_value(struct config_set *cs, const char *key, const char *value) +static int configset_add_value(struct config_reader *reader, + struct config_set *cs, const char *key, + const char *value) { struct config_set_element *e; struct string_list_item *si; @@ -2370,12 +2370,12 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha l_item->e = e; l_item->value_index = e->value_list.nr - 1; - if (!cf) + if (!reader->source) BUG("configset_add_value has no source"); - if (cf->name) { - kv_info->filename = strintern(cf->name); - kv_info->linenr = cf->linenr; - kv_info->origin_type = cf->origin_type; + if (reader->source->name) { + kv_info->filename = strintern(reader->source->name); + kv_info->linenr = reader->source->linenr; + kv_info->origin_type = reader->source->origin_type; } else { /* for values read from `git_config_from_parameters()` */ kv_info->filename = NULL; @@ -2430,16 +2430,25 @@ void git_configset_clear(struct config_set *cs) cs->list.items = NULL; } +struct configset_add_data { + struct config_set *config_set; + struct config_reader *config_reader; +}; +#define CONFIGSET_ADD_INIT { 0 } + static int config_set_callback(const char *key, const char *value, void *cb) { - struct config_set *cs = cb; - configset_add_value(cs, key, value); + struct configset_add_data *data = cb; + configset_add_value(data->config_reader, data->config_set, key, value); return 0; } int git_configset_add_file(struct config_set *cs, const char *filename) { - return git_config_from_file(config_set_callback, filename, cs); + struct configset_add_data data = CONFIGSET_ADD_INIT; + data.config_reader = &the_reader; + data.config_set = cs; + return git_config_from_file(config_set_callback, filename, &data); } int git_configset_get_value(struct config_set *cs, const char *key, const char **value) @@ -2554,6 +2563,7 @@ int git_configset_get_pathname(struct config_set *cs, const char *key, const cha static void repo_read_config(struct repository *repo) { struct config_options opts = { 0 }; + struct configset_add_data data = CONFIGSET_ADD_INIT; opts.respect_includes = 1; opts.commondir = repo->commondir; @@ -2565,8 +2575,10 @@ static void repo_read_config(struct repository *repo) git_configset_clear(repo->config); git_configset_init(repo->config); + data.config_set = repo->config; + data.config_reader = &the_reader; - if (config_with_options(config_set_callback, repo->config, NULL, &opts) < 0) + if (config_with_options(config_set_callback, &data, NULL, &opts) < 0) /* * config_with_options() normally returns only * zero, as most errors are fatal, and @@ -2692,9 +2704,12 @@ static void read_protected_config(void) .ignore_worktree = 1, .system_gently = 1, }; + struct configset_add_data data = CONFIGSET_ADD_INIT; + git_configset_init(&protected_config); - config_with_options(config_set_callback, &protected_config, - NULL, &opts); + data.config_set = &protected_config; + data.config_reader = &the_reader; + config_with_options(config_set_callback, &data, NULL, &opts); } void git_protected_config(config_fn_t fn, void *data) @@ -2879,6 +2894,7 @@ void git_die_config(const char *key, const char *err, ...) */ struct config_store_data { + struct config_reader *config_reader; size_t baselen; char *key; int do_not_match; @@ -2893,6 +2909,7 @@ struct config_store_data { unsigned int parsed_nr, parsed_alloc, *seen, seen_nr, seen_alloc; unsigned int key_seen:1, section_seen:1, is_keys_section:1; }; +#define CONFIG_STORE_INIT { 0 } static void config_store_data_clear(struct config_store_data *store) { @@ -2927,6 +2944,7 @@ static int store_aux_event(enum config_event_t type, size_t begin, size_t end, void *data) { struct config_store_data *store = data; + struct config_source *cs = store->config_reader->source; ALLOC_GROW(store->parsed, store->parsed_nr + 1, store->parsed_alloc); store->parsed[store->parsed_nr].begin = begin; @@ -2936,10 +2954,10 @@ static int store_aux_event(enum config_event_t type, if (type == CONFIG_EVENT_SECTION) { int (*cmpfn)(const char *, const char *, size_t); - if (cf->var.len < 2 || cf->var.buf[cf->var.len - 1] != '.') - return error(_("invalid section name '%s'"), cf->var.buf); + if (cs->var.len < 2 || cs->var.buf[cs->var.len - 1] != '.') + return error(_("invalid section name '%s'"), cs->var.buf); - if (cf->subsection_case_sensitive) + if (cs->subsection_case_sensitive) cmpfn = strncasecmp; else cmpfn = strncmp; @@ -2947,8 +2965,8 @@ static int store_aux_event(enum config_event_t type, /* Is this the section we were looking for? */ store->is_keys_section = store->parsed[store->parsed_nr].is_keys_section = - cf->var.len - 1 == store->baselen && - !cmpfn(cf->var.buf, store->key, store->baselen); + cs->var.len - 1 == store->baselen && + !cmpfn(cs->var.buf, store->key, store->baselen); if (store->is_keys_section) { store->section_seen = 1; ALLOC_GROW(store->seen, store->seen_nr + 1, @@ -3244,9 +3262,9 @@ int git_config_set_multivar_in_file_gently(const char *config_filename, char *filename_buf = NULL; char *contents = NULL; size_t contents_sz; - struct config_store_data store; + struct config_store_data store = CONFIG_STORE_INIT; - memset(&store, 0, sizeof(store)); + store.config_reader = &the_reader; /* parse-key returns negative; flip the sign to feed exit(3) */ ret = 0 - git_config_parse_key(key, &store.key, &store.baselen); From patchwork Wed Mar 1 00:38:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 13155393 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3AB6CC64EC4 for ; Wed, 1 Mar 2023 00:38:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229799AbjCAAih (ORCPT ); Tue, 28 Feb 2023 19:38:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41990 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229778AbjCAAiZ (ORCPT ); Tue, 28 Feb 2023 19:38:25 -0500 Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 40F761351C for ; Tue, 28 Feb 2023 16:38:24 -0800 (PST) Received: by mail-wm1-x32c.google.com with SMTP id r19-20020a05600c459300b003eb3e2a5e7bso5275386wmo.0 for ; Tue, 28 Feb 2023 16:38:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=vJ52iU4UDr/bFhJSTuiqwm9GxuCBT5aVR/KmFwpB5d4=; b=di1Xi/9S9/KtWn8EhV8dc/koSS0bKSRpnpvlapQVIfxtOpBNl9Q6fC97Odsh0wVR3J ODMjHW+4KlDho4XHNW1RvD+fdEtl0Ws2mVmyqeO7Oq1e05zY7jTKk+XrClFvXTwFlf+w ExTh4AO5y5UOCwM3Rzch3V0RoEXxqgD7Ft4SRIw4l5AFVpeNlgrD3psostR/H0ooZe2Y bO6oqu4tnO0PJJ6yX+xYc9LNaTCqWmJurlwntIIbfLnunHj4XoD+i+ISzTRAINxcZ87G YDKAOED/0TScQspG63iQgn3mBk+cfIGoIbr15ahRr2nyp4OIfSAcFY3oyjwPEKBeYFvq 40HQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vJ52iU4UDr/bFhJSTuiqwm9GxuCBT5aVR/KmFwpB5d4=; b=jdI8YV4pXHKfqns12SD89nXIu3U4cbpGFR3zhnFbZsSXGN1/bNbEiWsRrjXfnt1gos KbFIp7vCx6aou/JVZQPC0cG3n4T02TRiDf8SUZbz91cH3Gwlm6dd4ySDda8XWP55/RTV kCv4KUHFmilcHntyMLpVrGGfqIN4CJhGKhenUaeaiT8a6EfBTURQS5koFYlZjpD27Tdw u4zpc72MSB/kNARoKQWooJaQIg0TaphG51+BvCoehcnq2E1UFnbhSbqi6hlc9HD50ORJ zufYvsbPcUWaf6Hk3JJaUi+0YVaVnDSdTtOK7CbfZMisVblkqpNYUPtDzVgrFFO1b4pf EgFw== X-Gm-Message-State: AO0yUKVbKUQmh43eNxzlWbUgIDg1hZPHty5eYYl1Qu0/Wh9NkEDabOr3 QXU/NOkt/RvRf9XsCRgp98u+RKMuPfE= X-Google-Smtp-Source: AK7set+g2KPLgicxRjXCEvwfHRqh6Sr1dnbCpJNAaAms5HCRbssbUxEe55RjDezu7CyPcuRii8Ws/g== X-Received: by 2002:a05:600c:3549:b0:3eb:25ff:3446 with SMTP id i9-20020a05600c354900b003eb25ff3446mr3387633wmq.4.1677631102550; Tue, 28 Feb 2023 16:38:22 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id o25-20020a05600c511900b003dfe549da4fsm19611380wms.18.2023.02.28.16.38.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Feb 2023 16:38:22 -0800 (PST) Message-Id: In-Reply-To: References: Date: Wed, 01 Mar 2023 00:38:16 +0000 Subject: [PATCH 5/6] config.c: remove current_config_kvi Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jonathan Tan , Emily Shaffer , Jeff King , Derrick Stolee , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo Add ".config_kvi" to "struct config_reader" and replace "current_config_kvi" with "the_reader.config_kvi", either in-place (in public functions) or by passing "the_reader" to the "*data" arg of callback functions. Also, introduce a setter function for ".config_kvi", which allows us to enforce the contraint that only one of ".source" and ".config_kvi" can be set at a time (as documented in the comments). Because of this constraint, we know that "populate_remote_urls()" was never touching "current_config_kvi" when iterating through config files, so it doesn't need to store and restore that value. Signed-off-by: Glen Choo --- config.c | 103 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 61 insertions(+), 42 deletions(-) diff --git a/config.c b/config.c index 9676734b1b7..c7995148165 100644 --- a/config.c +++ b/config.c @@ -52,32 +52,28 @@ struct config_source { #define CONFIG_SOURCE_INIT { 0 } struct config_reader { + /* + * These members record the "current" config source, which can be + * accessed by parsing callbacks. + * + * The "source" variable will be non-NULL only when we are actually + * parsing a real config source (file, blob, cmdline, etc). + * + * The "config_kvi" variable will be non-NULL only when we are feeding + * cached config from a configset into a callback. + * + * They cannot be non-NULL at the same time. If they are both NULL, then + * we aren't parsing anything (and depending on the function looking at + * the variables, it's either a bug for it to be called in the first + * place, or it's a function which can be reused for non-config + * purposes, and should fall back to some sane behavior). + */ struct config_source *source; + struct key_value_info *config_kvi; }; /* Only public functions should reference the_reader. */ static struct config_reader the_reader; -/* - * FIXME The comments are temporarily out of date since "cf" been moved to - * the_reader, but not current_*. - * - * These variables record the "current" config source, which - * can be accessed by parsing callbacks. - * - * The "cf" variable will be non-NULL only when we are actually parsing a real - * config source (file, blob, cmdline, etc). - * - * The "current_config_kvi" variable will be non-NULL only when we are feeding - * cached config from a configset into a callback. - * - * They should generally never be non-NULL at the same time. If they are both - * NULL, then we aren't parsing anything (and depending on the function looking - * at the variables, it's either a bug for it to be called in the first place, - * or it's a function which can be reused for non-config purposes, and should - * fall back to some sane behavior). - */ -static struct key_value_info *current_config_kvi; - /* * Similar to the variables above, this gives access to the "scope" of the * current value (repo, global, etc). For cached values, it can be found via @@ -90,6 +86,8 @@ static enum config_scope current_parsing_scope; static inline void config_reader_push_source(struct config_reader *reader, struct config_source *top) { + if (reader->config_kvi) + BUG("source should only be set when parsing a config source"); if (reader->source) top->prev = reader->source; reader->source = top; @@ -105,6 +103,14 @@ static inline struct config_source *config_reader_pop_source(struct config_reade return ret; } +static inline void config_reader_set_kvi(struct config_reader *reader, + struct key_value_info *kvi) +{ + if (kvi && reader->source) + BUG("kvi should only be set when iterating through configset"); + reader->config_kvi = kvi; +} + static int pack_compression_seen; static int zlib_compression_seen; @@ -373,20 +379,17 @@ static void populate_remote_urls(struct config_include_data *inc) { struct config_options opts; - struct key_value_info *store_kvi = current_config_kvi; enum config_scope store_scope = current_parsing_scope; opts = *inc->opts; opts.unconditional_remote_url = 1; - current_config_kvi = NULL; current_parsing_scope = 0; inc->remote_urls = xmalloc(sizeof(*inc->remote_urls)); string_list_init_dup(inc->remote_urls); config_with_options(add_remote_url, inc->remote_urls, inc->config_source, &opts); - current_config_kvi = store_kvi; current_parsing_scope = store_scope; } @@ -2253,26 +2256,34 @@ int config_with_options(config_fn_t fn, void *data, return ret; } +struct configset_iter_data { + struct config_reader *config_reader; + void *inner; +}; +#define CONFIGSET_ITER_INIT { 0 } + static void configset_iter(struct config_set *cs, config_fn_t fn, void *data) { int i, value_index; struct string_list *values; struct config_set_element *entry; struct configset_list *list = &cs->list; + struct configset_iter_data *iter_data = data; for (i = 0; i < list->nr; i++) { + struct key_value_info *kvi; entry = list->items[i].e; value_index = list->items[i].value_index; values = &entry->value_list; - current_config_kvi = values->items[value_index].util; + kvi = values->items[value_index].util; + config_reader_set_kvi(iter_data->config_reader, kvi); - if (fn(entry->key, values->items[value_index].string, data) < 0) - git_die_config_linenr(entry->key, - current_config_kvi->filename, - current_config_kvi->linenr); + if (fn(entry->key, values->items[value_index].string, iter_data->inner) < 0) + git_die_config_linenr(entry->key, kvi->filename, + kvi->linenr); - current_config_kvi = NULL; + config_reader_set_kvi(iter_data->config_reader, NULL); } } @@ -2607,10 +2618,14 @@ static void repo_config_clear(struct repository *repo) git_configset_clear(repo->config); } -void repo_config(struct repository *repo, config_fn_t fn, void *data) +void repo_config(struct repository *repo, config_fn_t fn, void *data_inner) { + struct configset_iter_data data = CONFIGSET_ITER_INIT; + data.inner = data_inner; + data.config_reader = &the_reader; + git_config_check_init(repo); - configset_iter(repo->config, fn, data); + configset_iter(repo->config, fn, &data); } int repo_config_get_value(struct repository *repo, @@ -2712,11 +2727,15 @@ static void read_protected_config(void) config_with_options(config_set_callback, &data, NULL, &opts); } -void git_protected_config(config_fn_t fn, void *data) +void git_protected_config(config_fn_t fn, void *data_inner) { + struct configset_iter_data data = CONFIGSET_ITER_INIT; if (!protected_config.hash_initialized) read_protected_config(); - configset_iter(&protected_config, fn, data); + data.inner = data_inner; + data.config_reader = &the_reader; + + configset_iter(&protected_config, fn, &data); } /* Functions used historically to read configuration from 'the_repository' */ @@ -3823,8 +3842,8 @@ int parse_config_key(const char *var, const char *current_config_origin_type(void) { int type; - if (current_config_kvi) - type = current_config_kvi->origin_type; + if (the_reader.config_kvi) + type = the_reader.config_kvi->origin_type; else if(the_reader.source) type = the_reader.source->origin_type; else @@ -3869,8 +3888,8 @@ const char *config_scope_name(enum config_scope scope) const char *current_config_name(void) { const char *name; - if (current_config_kvi) - name = current_config_kvi->filename; + if (the_reader.config_kvi) + name = the_reader.config_kvi->filename; else if (the_reader.source) name = the_reader.source->name; else @@ -3880,16 +3899,16 @@ const char *current_config_name(void) enum config_scope current_config_scope(void) { - if (current_config_kvi) - return current_config_kvi->scope; + if (the_reader.config_kvi) + return the_reader.config_kvi->scope; else return current_parsing_scope; } int current_config_line(void) { - if (current_config_kvi) - return current_config_kvi->linenr; + if (the_reader.config_kvi) + return the_reader.config_kvi->linenr; else return the_reader.source->linenr; } From patchwork Wed Mar 1 00:38:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Glen Choo X-Patchwork-Id: 13155395 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F1F92C64EC4 for ; Wed, 1 Mar 2023 00:38:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229850AbjCAAil (ORCPT ); Tue, 28 Feb 2023 19:38:41 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42192 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229788AbjCAAib (ORCPT ); Tue, 28 Feb 2023 19:38:31 -0500 Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0D17D23338 for ; Tue, 28 Feb 2023 16:38:25 -0800 (PST) Received: by mail-wm1-x334.google.com with SMTP id j3so7564158wms.2 for ; Tue, 28 Feb 2023 16:38:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=MwAlrsRdQ9AV0+EcvWx/2rheoExK0e6UCtNMmDBBOlg=; b=pSYD/zgNE1Gb1HIsK7wT+DO6Ns1Cc8PBDf0bveedPCNZ4RCj9F6ukQr12tpmqtDNFe jeTA/wdfoNL4GzW8ivVzQcQXmLE8Dj+odZsyJFelGic4wUXyc3pNJ1AjQ7LGW44IZZ8h pxmr7yR6Ptbpc6kqrLSrnUAaMZvdcDS4Kx7TGRqZ+C7MgWbK/An+COnUBu6Zj9GUDnGN /eOER5L7lL7o+i6Z4GNps5fKbcTjjCf33VcwzoB4kYldaGyZkE+0h20EeAaZlsbsJJ5R Dq1HLBeEho39f/7SduPuSo2Rw2Cht/QgCi3WJ5GIeZtoJ0ylrFYZOU1FWUg9Tegu5MXP sVfQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=MwAlrsRdQ9AV0+EcvWx/2rheoExK0e6UCtNMmDBBOlg=; b=HNyAu65FDviKlxv5geSOjQkj5QyOAMrEbCUkqd1RorGJ0Mn2BkRUri6WnrrvMHevZg SN7vkVQIuqj3jk6jK+WaC77h6cnQVcO9NDBNac68huQduqM6FyEY0WbbI7dUmG2eisq9 +YaAUHrux5M5ueVVfHhVb4QFQ6HnYVD2CPzWVgXdkalIJ0IEWOo60uSrMyvpxQ0h2F+t hh4L8lYfhAq92l1fUTvWFOiCWJwYmmZ6sDJyWtHbSIu8PsgZ4Pk0Mgz6dYCfVfZEZHUR 5xi5YsWNQz51NbR1sGspVkfK0SzqLLC6hMOTYWaS9K6Id3bC5nFAsHLeN172+3knKhoR Zvgw== X-Gm-Message-State: AO0yUKUwh6rs0TU3HI1jawpEMhae4NUro4ReQsAbNGssOo+tlKnmNEnd rcrw9YoFdQDJK3qf0Eh0engd5Ei4cBI= X-Google-Smtp-Source: AK7set/KS282ahPF5AQXelQN70zJXzTUAXP5GrGtgMou5xFc0ExJ+6AEZ3cAckhyvttWvYxLOJqDiQ== X-Received: by 2002:a05:600c:3b99:b0:3ea:fc95:7bf with SMTP id n25-20020a05600c3b9900b003eafc9507bfmr3275873wms.30.1677631103397; Tue, 28 Feb 2023 16:38:23 -0800 (PST) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id 13-20020a05600c020d00b003dc1d668866sm17559787wmi.10.2023.02.28.16.38.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Feb 2023 16:38:22 -0800 (PST) Message-Id: In-Reply-To: References: Date: Wed, 01 Mar 2023 00:38:17 +0000 Subject: [PATCH 6/6] config.c: remove current_parsing_scope Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Jonathan Tan , Emily Shaffer , Jeff King , Derrick Stolee , Glen Choo , Glen Choo Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Glen Choo From: Glen Choo Add ".parsing_scope" to "struct config_reader" and replace "current_parsing_scope" with "the_reader.parsing_scope. Adjust the comment slightly to make it clearer that the scope applies to the config source (not the current value), and should only be set when parsing a config source. As such, ".parsing_scope" (only set when parsing config sources) and ".config_kvi" (only set when iterating a config set) should not be set together, so enforce this with a setter function. Unlike previous commits, "populate_remote_urls()" still needs to store and restore the 'scope' value because it could have touched "current_parsing_scope" ("config_with_options()" can set the scope). Signed-off-by: Glen Choo --- config.c | 62 +++++++++++++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/config.c b/config.c index c7995148165..19bab84c47f 100644 --- a/config.c +++ b/config.c @@ -70,19 +70,20 @@ struct config_reader { */ struct config_source *source; struct key_value_info *config_kvi; + /* + * The "scope" of the current config source being parsed (repo, global, + * etc). Like "source", this is only set when parsing a config source. + * It's not part of "source" because it transcends a single file (i.e., + * a file included from .git/config is still in "repo" scope). + * + * When iterating through a configset, the equivalent value is + * "config_kvi.scope" (see above). + */ + enum config_scope parsing_scope; }; /* Only public functions should reference the_reader. */ static struct config_reader the_reader; -/* - * Similar to the variables above, this gives access to the "scope" of the - * current value (repo, global, etc). For cached values, it can be found via - * the current_config_kvi as above. During parsing, the current value can be - * found in this variable. It's not part of "cf" because it transcends a single - * file (i.e., a file included from .git/config is still in "repo" scope). - */ -static enum config_scope current_parsing_scope; - static inline void config_reader_push_source(struct config_reader *reader, struct config_source *top) { @@ -106,11 +107,19 @@ static inline struct config_source *config_reader_pop_source(struct config_reade static inline void config_reader_set_kvi(struct config_reader *reader, struct key_value_info *kvi) { - if (kvi && reader->source) + if (kvi && (reader->source || reader->parsing_scope)) BUG("kvi should only be set when iterating through configset"); reader->config_kvi = kvi; } +static inline void config_reader_set_scope(struct config_reader *reader, + enum config_scope scope) +{ + if (scope && reader->config_kvi) + BUG("scope should only be set when iterating through a config source"); + reader->parsing_scope = scope; +} + static int pack_compression_seen; static int zlib_compression_seen; @@ -379,18 +388,18 @@ static void populate_remote_urls(struct config_include_data *inc) { struct config_options opts; - enum config_scope store_scope = current_parsing_scope; + enum config_scope store_scope = inc->config_reader->parsing_scope; opts = *inc->opts; opts.unconditional_remote_url = 1; - current_parsing_scope = 0; + config_reader_set_scope(inc->config_reader, 0); inc->remote_urls = xmalloc(sizeof(*inc->remote_urls)); string_list_init_dup(inc->remote_urls); config_with_options(add_remote_url, inc->remote_urls, inc->config_source, &opts); - current_parsing_scope = store_scope; + config_reader_set_scope(inc->config_reader, store_scope); } static int forbid_remote_url(const char *var, const char *value UNUSED, @@ -2155,7 +2164,8 @@ int git_config_system(void) return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0); } -static int do_git_config_sequence(const struct config_options *opts, +static int do_git_config_sequence(struct config_reader *reader, + const struct config_options *opts, config_fn_t fn, void *data) { int ret = 0; @@ -2163,7 +2173,7 @@ static int do_git_config_sequence(const struct config_options *opts, char *xdg_config = NULL; char *user_config = NULL; char *repo_config; - enum config_scope prev_parsing_scope = current_parsing_scope; + enum config_scope prev_parsing_scope = reader->parsing_scope; if (opts->commondir) repo_config = mkpathdup("%s/config", opts->commondir); @@ -2172,13 +2182,13 @@ static int do_git_config_sequence(const struct config_options *opts, else repo_config = NULL; - current_parsing_scope = CONFIG_SCOPE_SYSTEM; + config_reader_set_scope(reader, CONFIG_SCOPE_SYSTEM); if (git_config_system() && system_config && !access_or_die(system_config, R_OK, opts->system_gently ? ACCESS_EACCES_OK : 0)) ret += git_config_from_file(fn, system_config, data); - current_parsing_scope = CONFIG_SCOPE_GLOBAL; + config_reader_set_scope(reader, CONFIG_SCOPE_GLOBAL); git_global_config(&user_config, &xdg_config); if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) @@ -2187,12 +2197,12 @@ static int do_git_config_sequence(const struct config_options *opts, if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) ret += git_config_from_file(fn, user_config, data); - current_parsing_scope = CONFIG_SCOPE_LOCAL; + config_reader_set_scope(reader, CONFIG_SCOPE_LOCAL); if (!opts->ignore_repo && repo_config && !access_or_die(repo_config, R_OK, 0)) ret += git_config_from_file(fn, repo_config, data); - current_parsing_scope = CONFIG_SCOPE_WORKTREE; + config_reader_set_scope(reader, CONFIG_SCOPE_WORKTREE); if (!opts->ignore_worktree && repository_format_worktree_config) { char *path = git_pathdup("config.worktree"); if (!access_or_die(path, R_OK, 0)) @@ -2200,11 +2210,11 @@ static int do_git_config_sequence(const struct config_options *opts, free(path); } - current_parsing_scope = CONFIG_SCOPE_COMMAND; + config_reader_set_scope(reader, CONFIG_SCOPE_COMMAND); if (!opts->ignore_cmdline && git_config_from_parameters(fn, data) < 0) die(_("unable to parse command-line config")); - current_parsing_scope = prev_parsing_scope; + config_reader_set_scope(reader, prev_parsing_scope); free(system_config); free(xdg_config); free(user_config); @@ -2217,6 +2227,7 @@ int config_with_options(config_fn_t fn, void *data, const struct config_options *opts) { struct config_include_data inc = CONFIG_INCLUDE_INIT; + enum config_scope prev_scope = the_reader.parsing_scope; int ret; if (opts->respect_includes) { @@ -2230,7 +2241,7 @@ int config_with_options(config_fn_t fn, void *data, } if (config_source) - current_parsing_scope = config_source->scope; + config_reader_set_scope(&the_reader, config_source->scope); /* * If we have a specific filename, use it. Otherwise, follow the @@ -2246,13 +2257,14 @@ int config_with_options(config_fn_t fn, void *data, ret = git_config_from_blob_ref(fn, repo, config_source->blob, data); } else { - ret = do_git_config_sequence(opts, fn, data); + ret = do_git_config_sequence(&the_reader, opts, fn, data); } if (inc.remote_urls) { string_list_clear(inc.remote_urls, 0); FREE_AND_NULL(inc.remote_urls); } + config_reader_set_scope(&the_reader, prev_scope); return ret; } @@ -2393,7 +2405,7 @@ static int configset_add_value(struct config_reader *reader, kv_info->linenr = -1; kv_info->origin_type = CONFIG_ORIGIN_CMDLINE; } - kv_info->scope = current_parsing_scope; + kv_info->scope = reader->parsing_scope; si->util = kv_info; return 0; @@ -3902,7 +3914,7 @@ enum config_scope current_config_scope(void) if (the_reader.config_kvi) return the_reader.config_kvi->scope; else - return current_parsing_scope; + return the_reader.parsing_scope; } int current_config_line(void)