All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Rubén Justo" <rjusto@gmail.com>
Subject: [PATCH 09/11] merge: use git_config_string_dup() for pull strategies
Date: Sat, 6 Apr 2024 21:04:29 -0400	[thread overview]
Message-ID: <20240407010429.GI868358@coredump.intra.peff.net> (raw)
In-Reply-To: <20240407005656.GA436890@coredump.intra.peff.net>

Converting pull.twohead and pull.octopus to use git_config_string_dup()
fixes possible leaks if we see those config variables defined multiple
times. Doing so is mostly an "easy" case, except that we later may
assign a string literal to pull_twohead (which is now a non-const
pointer).

That's actually not _too_ bad in practice, as it happens after we've
done all of our config parsing (so nobody would try to free it). And the
compiler won't complain unless -Wwrite-strings is used (and turning that
on creates a host of other warnings, some useful and some not). But
while we're here let's future proof it as best we can.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/merge.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index c2be29ed2f..a9e5100e70 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -101,7 +101,7 @@ static struct strategy all_strategy[] = {
 	{ "subtree",    NO_FAST_FORWARD | NO_TRIVIAL },
 };
 
-static const char *pull_twohead, *pull_octopus;
+static char *pull_twohead, *pull_octopus;
 
 enum ff_type {
 	FF_NO,
@@ -615,9 +615,9 @@ static int git_merge_config(const char *k, const char *v,
 	else if (!strcmp(k, "merge.verifysignatures"))
 		verify_signatures = git_config_bool(k, v);
 	else if (!strcmp(k, "pull.twohead"))
-		return git_config_string(&pull_twohead, k, v);
+		return git_config_string_dup(&pull_twohead, k, v);
 	else if (!strcmp(k, "pull.octopus"))
-		return git_config_string(&pull_octopus, k, v);
+		return git_config_string_dup(&pull_octopus, k, v);
 	else if (!strcmp(k, "commit.cleanup"))
 		return git_config_string_dup(&cleanup_arg, k, v);
 	else if (!strcmp(k, "merge.ff")) {
@@ -1291,7 +1291,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (!pull_twohead) {
 		char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
 		if (default_strategy && !strcmp(default_strategy, "ort"))
-			pull_twohead = "ort";
+			pull_twohead = xstrdup("ort");
 	}
 
 	init_diff_ui_defaults();
-- 
2.44.0.872.g288abe5b5b


  parent reply	other threads:[~2024-04-07  1:04 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-06 18:11 [PATCH] config: do not leak excludes_file Junio C Hamano
2024-04-06 19:17 ` [WIP] git_config_pathname() leakfix Junio C Hamano
2024-04-07  0:56 ` [PATCH 0/12] git_config_string() considered harmful Jeff King
2024-04-07  0:58   ` [PATCH 01/11] config: make sequencer.c's git_config_string_dup() public Jeff King
2024-04-07  1:00   ` [PATCH 02/11] config: add git_config_pathname_dup() Jeff King
2024-04-07  1:00   ` [PATCH 03/11] config: prefer git_config_string_dup() for temp variables Jeff King
2024-04-07  1:50     ` Eric Sunshine
2024-04-07  2:45       ` Jeff King
2024-04-07  1:01   ` [PATCH 04/11] config: use git_config_string_dup() for open-coded equivalents Jeff King
2024-04-07  1:01   ` [PATCH 05/11] config: use git_config_string_dup() to fix leaky open coding Jeff King
2024-04-07  1:02   ` [PATCH 06/11] config: use git_config_string() in easy cases Jeff King
2024-04-07  2:05     ` Eric Sunshine
2024-04-07  2:47       ` Jeff King
2024-04-07  1:03   ` [PATCH 07/11] config: use git_config_pathname_dup() " Jeff King
2024-04-07  1:03   ` [PATCH 08/11] http: use git_config_string_dup() Jeff King
2024-04-07  1:04   ` Jeff King [this message]
2024-04-07  1:04   ` [PATCH 10/11] userdiff: use git_config_string_dup() when we can Jeff King
2024-04-07  1:07   ` [PATCH 11/11] blame: use "dup" string_list for ignore-revs files Jeff King
2024-04-07  2:42     ` Eric Sunshine
2024-04-07  1:08   ` [PATCH 0/12] git_config_string() considered harmful Jeff King
2024-04-07 17:58   ` Rubén Justo
2024-04-08 20:55     ` Jeff King
2024-04-11 23:36       ` Rubén Justo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240407010429.GI868358@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=rjusto@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.