git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/6] commit: copy saved getenv() result

From
Jeff King <peff@peff.net>
Date
Jan 11, 2019, 22:15 UTC
Message-ID
<20190111221539.GB10188@sigill.intra.peff.net>
In-Reply-To
<20190111221414.GA31335@sigill.intra.peff.net>

We save the result of $GIT_INDEX_FILE so that we can restore it after setting it to a new value and running add--interactive. However, the pointer returned by getenv() is not guaranteed to be valid after calling setenv(). This _usually_ works fine, but can fail if libc needs to reallocate the environment block during the setenv().

Let's just duplicate the string, so we know that it remains valid.

In the long run it may be more robust to teach interactive_add() to take a set of environment variables to pass along to run-command when it execs add--interactive. And then we would not have to do this save/restore dance at all. But this is an easy fix in the meantime.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/commit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 004b816635..7d2e0b61e5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -351,7 +351,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		if (write_locked_index(&the_index, &index_lock, 0))
 			die(_("unable to create temporary index"));
 
-		old_index_env = getenv(INDEX_ENVIRONMENT);
+		old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT));
 		setenv(INDEX_ENVIRONMENT, get_lock_file_path(&index_lock), 1);
 
 		if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
@@ -361,6 +361,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 			setenv(INDEX_ENVIRONMENT, old_index_env, 1);
 		else
 			unsetenv(INDEX_ENVIRONMENT);
+		FREE_AND_NULL(old_index_env);
 
 		discard_cache();
 		read_cache_from(get_lock_file_path(&index_lock));
-- 
2.20.1.651.g2d41a78c67
Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 26 in “getenv() timing fixes”
  1. 0/6 getenv() timing fixesJeff King, Jan 11, 2019
  2. 1/6 get_super_prefix(): copy getenv() resultJeff King, Jan 11, 2019
  3. Junio C HamanoJan 12, 2019
  4. 2/6 commit: copy saved getenv() resultJeff King, Jan 11, 2019
  5. Junio C HamanoJan 12, 2019
  6. Jeff KingJan 12, 2019
  7. Johannes SchindelinJan 15, 2019
  8. Jeff KingJan 15, 2019
  9. Stefan BellerJan 15, 2019
  10. Jeff KingJan 15, 2019
  11. Johannes SchindelinJan 16, 2019
  12. 3/6 config: make a copy of $GIT_CONFIG stringJeff King, Jan 11, 2019
  13. 4/6 init: make a copy of $GIT_DIR stringJeff King, Jan 11, 2019
  14. Junio C HamanoJan 12, 2019
  15. 5/6 merge-recursive: copy $GITHEAD stringsJeff King, Jan 11, 2019
  16. Junio C HamanoJan 12, 2019
  17. 6/6 builtin_diff(): read $GIT_DIFF_OPTS closer to useJeff King, Jan 11, 2019
  18. Ævar Arnfjörð BjarmasonJan 12, 2019
  19. Stefan BellerJan 12, 2019
  20. Jeff KingJan 15, 2019
  21. Junio C HamanoJan 15, 2019
  22. Stefan BellerJan 15, 2019
  23. Jeff KingJan 15, 2019
  24. Jeff KingJan 15, 2019
  25. Junio C HamanoJan 15, 2019
  26. Jeff KingJan 15, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.