From patchwork Fri May 3 09:56:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13652597 Received: from wfhigh8-smtp.messagingengine.com (wfhigh8-smtp.messagingengine.com [64.147.123.159]) (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 145D414F9D8 for ; Fri, 3 May 2024 09:56:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.123.159 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714730212; cv=none; b=h8llbpFGoTtD8/yyN8qjqOTLzQLA+Bx166SC4ullQsLCzHaY5/Tx6GK5/GtxmuDJMDe+58K6ls+0+HwpirollB+uMXtkJ1AG6hw9dK7CHWGFRFwGyQiq3Mqv44TD+8vVQhkfBOa94Q3FxuR7tKqRRyFhuIolHGuYzTUK7KLhtR8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714730212; c=relaxed/simple; bh=+9U3DQvT+Pju05MkyfgjV7Sp8n1yKy8b8ChMhCnC6F4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=nnEQlHa0qHoJQw7oDXYJnk5msUYKE/q3qoquE01WGCHJRx1ZuyWth+8SNykZSjxe15Zq3Irw645X/fgUDeD24P2GkqH6/R/0c7KEDjC3dYIUWw9oyWeM7Nl6d/YlOGQ7/Jz7seEO8ridA1Tth4GQOhNmNRDMLhLjalAw68SV4/I= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none 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=Hl3/4HX5; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=B0dLcvad; arc=none smtp.client-ip=64.147.123.159 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none 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="Hl3/4HX5"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="B0dLcvad" Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailfhigh.west.internal (Postfix) with ESMTP id 067B31800137; Fri, 3 May 2024 05:56:49 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Fri, 03 May 2024 05:56:50 -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=1714730209; x=1714816609; bh=2tjCFY5VRp SyCzTR3kEnvxi9Up7DyGJsFuwOjMEEID4=; b=Hl3/4HX5LJgszE3c2YeUYPl3Jz UIcGh0opW/sdaRFftIA18z94/YyxBhxFjDFHodObAv3gOD5hIVj6jTlcyz4EyXqH 02QZzkW7QRskP8491+im/zodLVsxXp2XntqooBLL687i0ZazDPwh3ECNjmtK0DY9 Btc8K60gIHnHZw7a0jXq3a602GKQnYKWgV9wuMjCfEFvK5YE/b/KrG+7/aZmnjhM Ylgh/faDmw83JckaNM6CiRaTovwbDjzCbTwZSGajqNoRxoDpRPI6GYlAuFz2Y3Bu SG5o1UkVzO8Y6HQeDr80RrbA/15jW1fp2/vJIvaDFD4pWgRt6Rp7o+/wFr5A== 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=1714730209; x=1714816609; bh=2tjCFY5VRpSyCzTR3kEnvxi9Up7D yGJsFuwOjMEEID4=; b=B0dLcvadMHMBTQiw9VKS40h2uqZsuj9yBNCVB4r7hNs/ oQDG4kaAY8FtujHVDWeBq376ESAzzRB1Zxd6lD+GqWvJ1hSxVepa5ovNkX5hZkKT 1iJT+wwBlmFDixevCx7vHGH9B47zytLKUG4zQmMpfqfEl7059EJyX89Hg3n1OkVs PU2Myu3PltE+Dmno2VoLNUaX4YIVJSUpzFHoNauhlCn953R/O5HVbEh9PVD/1ef/ cC+xPH6w646PfoOCd+HgtnCOFMyOXYZCMboe+BwOsEukINZ3C9wacYia7Yyuc8El YsaEdjPtYa0oNoulcXQjPPVDhGM+xOxJFwmRc6jXQw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrvddvtddgvddtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 3 May 2024 05:56:48 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 847dcd60 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Fri, 3 May 2024 09:56:19 +0000 (UTC) Date: Fri, 3 May 2024 11:56:44 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Kristoffer Haugsbakk , Taylor Blau , =?iso-8859-1?q?Jean-No=EBl?= AVILA , Eric Sunshine , Junio C Hamano Subject: [PATCH v4 01/14] config: clarify memory ownership when preparing comment strings Message-ID: <3aa26d5bff21e0b93736b1502fa90694f6b2cedc.1714730169.git.ps@pks.im> 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: The ownership of memory returned when preparing a comment string is quite intricate: when the returned value is different than the passed value, then the caller is responsible to free the memory. This is quite subtle, and it's even easier to miss because the returned value is in fact a `const char *`. Adapt the function to always return either `NULL` or a newly allocated string. The function is called at most once per git-config(1), so it's not like this micro-optimization really matters. Thus, callers are now always responsible for freeing the value. --- builtin/config.c | 11 ++++++----- config.c | 16 ++++++---------- config.h | 2 +- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 0015620dde..40456c0770 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -44,7 +44,7 @@ static struct config_options config_options; static int show_origin; static int show_scope; static int fixed_value; -static const char *comment; +static const char *comment_arg; #define ACTION_GET (1<<0) #define ACTION_GET_ALL (1<<1) @@ -174,7 +174,7 @@ static struct option builtin_config_options[] = { OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")), OPT_BOOL(0, "show-scope", &show_scope, N_("show scope of config (worktree, local, global, system, command)")), OPT_STRING(0, "default", &default_value, N_("value"), N_("with --get, use default value when missing entry")), - OPT_STRING(0, "comment", &comment, N_("value"), N_("human-readable comment string (# will be prepended as needed)")), + OPT_STRING(0, "comment", &comment_arg, N_("value"), N_("human-readable comment string (# will be prepended as needed)")), OPT_END(), }; @@ -674,7 +674,7 @@ static char *default_user_config(void) int cmd_config(int argc, const char **argv, const char *prefix) { int nongit = !startup_info->have_repository; - char *value = NULL; + char *value = NULL, *comment = NULL; int flags = 0; int ret = 0; struct key_value_info default_kvi = KVI_INIT; @@ -799,7 +799,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_builtin_config(); } - if (comment && + if (comment_arg && !(actions & (ACTION_ADD|ACTION_SET|ACTION_SET_ALL|ACTION_REPLACE_ALL))) { error(_("--comment is only applicable to add/set/replace operations")); usage_builtin_config(); @@ -841,7 +841,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) flags |= CONFIG_FLAGS_FIXED_VALUE; } - comment = git_config_prepare_comment_string(comment); + comment = git_config_prepare_comment_string(comment_arg); if (actions & PAGING_ACTIONS) setup_auto_pager("config", 1); @@ -993,6 +993,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) return get_colorbool(argv[0], argc == 2); } + free(comment); free(value); return ret; } diff --git a/config.c b/config.c index ae3652b08f..13cf9eeb16 100644 --- a/config.c +++ b/config.c @@ -3182,14 +3182,10 @@ void git_config_set(const char *key, const char *value) trace2_cmd_set_config(key, value); } -/* - * The ownership rule is that the caller will own the string - * if it receives a piece of memory different from what it passed - * as the parameter. - */ -const char *git_config_prepare_comment_string(const char *comment) +char *git_config_prepare_comment_string(const char *comment) { size_t leading_blanks; + char *prepared; if (!comment) return NULL; @@ -3210,13 +3206,13 @@ const char *git_config_prepare_comment_string(const char *comment) leading_blanks = strspn(comment, " \t"); if (leading_blanks && comment[leading_blanks] == '#') - ; /* use it as-is */ + prepared = xstrdup(comment); /* use it as-is */ else if (comment[0] == '#') - comment = xstrfmt(" %s", comment); + prepared = xstrfmt(" %s", comment); else - comment = xstrfmt(" # %s", comment); + prepared = xstrfmt(" # %s", comment); - return comment; + return prepared; } static void validate_comment_string(const char *comment) diff --git a/config.h b/config.h index f4966e3749..db8b608064 100644 --- a/config.h +++ b/config.h @@ -338,7 +338,7 @@ void git_config_set_multivar(const char *, const char *, const char *, unsigned) int repo_config_set_multivar_gently(struct repository *, const char *, const char *, const char *, unsigned); int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, const char *, unsigned); -const char *git_config_prepare_comment_string(const char *); +char *git_config_prepare_comment_string(const char *); /** * takes four parameters: