From patchwork Mon May 13 10:21:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13663256 Received: from wfhigh6-smtp.messagingengine.com (wfhigh6-smtp.messagingengine.com [64.147.123.157]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 175AC146D76 for ; Mon, 13 May 2024 10:22:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.157 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715595725; cv=none; b=BJoi8itkyY/CVzNDNUXALauUs2Rj+xuDUk4Qf1DMiJ7BoFVYDzLBWiXbYgp3DuL4pN2Ls3G9+36XQzmpJG8ZQdGohRHqZ6DV0eyGeY4agld4P8WZXKH+8Gi+sGLS0aWlHgSiEohMAf/6iAa/IBQ9QE7T7qJDqoGgHkjPCZdc5AU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715595725; c=relaxed/simple; bh=k/9KdUE2BRmVR8/pt2+jmX0E4OX6qYXrM86bKCS2a3c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LY3+Z8DyqAvHILzZJ3JYHPiBMV1nDr7KuKe3P0vImFaVqhE+9/Ev1Oa3G0ViR1mEzi5B8dwdqawXmLrKjwws1Le0EGIriGiKsLQ65Y4ekoJtuKNqKOQKwhcj9hwjjH9QkOcSwlCNwKgqeX5AizJtaQrdGmZihJBnRzEyxgMtgGM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=DnlljjTO; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=F1e7PbJB; arc=none smtp.client-ip=64.147.123.157 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="DnlljjTO"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="F1e7PbJB" Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfhigh.west.internal (Postfix) with ESMTP id 09D9E1800076; Mon, 13 May 2024 06:22:02 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Mon, 13 May 2024 06:22:03 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1715595722; x=1715682122; bh=/0zq9R9/EJ ett9kUp9kShmtP1pfSlbrGRXfO5UepYT4=; b=DnlljjTOxsjTCNj7vUjeUKx3Wr D9mI9yFM3ZX3ljknDMmo7ptq7vrg0y3IoXgkli2GeKjxCzGnQt79gUOwfFo7qLuz fWrWyRozlBaiLh+4O6sGoefejE0ZowHSrLJkKsDcdKFT4ZPdfqAXmZKr3Uas888f xRGQxurmOwCOEBXSNnkuTwJhUUqQo56MTadcx2Yt0tUhQf5H6UeU4P6VsU7dLyWd MxiB8tIlMNJB85MKXr/1c7tiC/6yMZu8U4sSc/ycKO5Uy2ii6/ypsyri0s2D7zhh obBAer239i/+t0zRLHSsDQ19Ti62Kel85COw15+ERZTMMjpT3RgXvUmzWB5g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1715595722; x=1715682122; bh=/0zq9R9/EJett9kUp9kShmtP1pfS lbrGRXfO5UepYT4=; b=F1e7PbJBKLWjy7bLJSnF0ra9/FTgvto/a9a8A38htiBw aGtYfln1XCcPUKapsY42vO16eH0vA4NXD9q+z86KXkJHXKaMEXxANmw4JvNzuS8D 5QpPvGSmlPuZl1/au8ncEl9tE6kf56hLXUulFiLi9YQCeFH5fmobTkO3ChRz/OrR gXNSbhKuX4qZBiWf/CSKEjmYo1xpveZb9sq11jHpAStmYkmx9XE5StpSpZtHirlU mCKcxqc112mWoSZ+kBZTVhAcJ4Y6VtNytj45Jv9Jy9WdpHpUF07Feqb1XckA1PgX XRSvqdteWtibeWeQIaYRFZcn7x5SBXjfS48iSba2zA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrvdeggedgvdejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 13 May 2024 06:22:01 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id b867f8f3 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 13 May 2024 10:21:42 +0000 (UTC) Date: Mon, 13 May 2024 12:21:58 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Junio C Hamano , Kyle Lippincott , Eric Sunshine Subject: [PATCH v2 00/21] builtin/config: remove global state Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Hi, this is the second version of my patch series that removes global state from "builtin/config.c". Changes compared to v1: - Reinstated a comment in patch 5. - Fixed a memory leak in patch 9. - A couple of commit message fixes. The series continues to build on top of ps/config-subcommands. Thanks! Patrick Patrick Steinhardt (21): builtin/config: stop printing full usage on misuse builtin/config: move legacy mode into its own function builtin/config: move subcommand options into `cmd_config()` builtin/config: move legacy options into `cmd_config()` builtin/config: move actions into `cmd_config_actions()` builtin/config: check for writeability after source is set up config: make the config source const builtin/config: refactor functions to have common exit paths builtin/config: move location options into local variables builtin/config: move display options into local variables builtin/config: move type options into display options builtin/config: move default value into display options builtin/config: move `respect_includes_opt` into location options builtin/config: convert `do_not_match` to a local variable builtin/config: convert `value_pattern` to a local variable builtin/config: convert `regexp` to a local variable builtin/config: convert `key_regexp` to a local variable builtin/config: convert `key` to a local variable builtin/config: track "fixed value" option via flags only builtin/config: convert flags to a local variable builtin/config: pass data between callbacks via local variables builtin/config.c | 968 ++++++++++++++++++++++++++-------------------- config.c | 4 +- config.h | 2 +- t/t1300-config.sh | 9 +- 4 files changed, 550 insertions(+), 433 deletions(-) Range-diff against v1: 1: 0ba7628126 = 1: 0ba7628126 builtin/config: stop printing full usage on misuse 2: 663e1f74f8 = 2: 663e1f74f8 builtin/config: move legacy mode into its own function 3: 1239c151d0 = 3: 1239c151d0 builtin/config: move subcommand options into `cmd_config()` 4: 82964510c5 = 4: 82964510c5 builtin/config: move legacy options into `cmd_config()` 5: 2e308393ed ! 5: 0a6ecae2cc builtin/config: move actions into `cmd_config_actions()` @@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, con comment = git_config_prepare_comment_string(comment_arg); - if (actions & PAGING_ACTIONS) ++ /* ++ * The following actions may produce more than one line of output and ++ * should therefore be paged. ++ */ + if (actions & (ACTION_LIST | ACTION_GET_ALL | ACTION_GET_REGEXP | ACTION_GET_URLMATCH)) setup_auto_pager("config", 1); 6: edfd7caa39 = 6: 7ab99a27c1 builtin/config: check for writeability after source is set up 7: bfba68aa1e = 7: 1460d3a36c config: make the config source const 8: 6bff0410e9 = 8: 018ed0226b builtin/config: refactor functions to have common exit paths 9: a96c122280 ! 9: b5d43b6f85 builtin/config: move location options into local variables @@ builtin/config.c: static char *default_user_config(void) - given_config_source.file = git_global_config(); - if (!given_config_source.file) + if (opts->use_global_config) { -+ opts->source.file = xstrdup_or_null(git_global_config()); ++ opts->source.file = git_global_config(); + if (!opts->source.file) /* * It is unknown if HOME/.gitconfig exists, so 10: 06c1e08fc4 = 10: d66e14af30 builtin/config: move display options into local variables 11: 9610897662 = 11: 63436c3416 builtin/config: move type options into display options 12: eb79462861 = 12: 106b8ac8a2 builtin/config: move default value into display options 13: b9ebfbd667 = 13: 8a6b555b58 builtin/config: move `respect_includes_opt` into location options 14: 2b40b784fe ! 14: 0dd22bf51a builtin/config: convert `do_not_match` to a local variable @@ Commit message builtin/config: convert `do_not_match` to a local variable The `do_not_match` variable is used by the `format_config()` callback as - an indicator whteher or not the passed regular expression is negated. It + an indicator whether or not the passed regular expression is negated. It is only ever set up by its only caller, `collect_config()` and can thus easily be moved into the `collect_config_data` structure. 15: 71d1b7a51b = 15: b656951f0c builtin/config: convert `value_pattern` to a local variable 16: c3b340f119 = 16: b56a07bda0 builtin/config: convert `regexp` to a local variable 17: 835cc0acfb = 17: 323cb05120 builtin/config: convert `key_regexp` to a local variable 18: 2aee7ec5d8 ! 18: e972e63be8 builtin/config: convert `key` to a local variable @@ Commit message The `key` variable is used by the `get_value()` function for two purposes: - - It is used to store the result of `git_config_parse_key()`, which si + - It is used to store the result of `git_config_parse_key()`, which is then passed on to `collect_config()`. - It is used as a store to convert the provided key to an all-lowercase key when `use_key_regexp` is set. - Both of these cases don't warrant a global variable at all. In the - former case we can pass the key via `struct collect_config_data`. And in - the latter case we really only want to have it as a temporary local - variable such that we can free associated memory. + Neither of these cases warrant a global variable at all. In the former + case we can pass the key via `struct collect_config_data`. And in the + latter case we really only want to have it as a temporary local variable + such that we can free associated memory. Refactor the code accordingly to reduce our reliance on global state. 19: 625216a774 ! 19: d83c3d085e builtin/config: track "fixed value" option via flags only @@ Commit message is not aware that this is tracked via two separate mechanisms. Refactor the code to use the flag exclusively. We already pass it to all - the require callsites anyway, except for `collect_config()`. + the required callsites anyway, except for `collect_config()`. Signed-off-by: Patrick Steinhardt 20: 05254d512b = 20: 294bcd96a4 builtin/config: convert flags to a local variable 21: 3a5f059789 = 21: 0496b958e2 builtin/config: pass data between callbacks via local variables