[PATCH v2 0/4] Fix bugs related to real_path()

From: "Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Subject: [PATCH v2 0/4] Fix bugs related to real_path()
Date: Tue, 10 Mar 2020 13:11:20 +0000	[thread overview]
Message-ID: <pull.575.v2.git.1583845884.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.575.git.1583521396.gitgitgadget@gmail.com>

Changes since V1
-------------------
1) Removed `strbuf_realpath()` that weren't needed
2) Code style in declaration of `get_superproject_working_tree()`

Original description
-------------------
The issue with `real_path()` seems to be long-standing, where multiple
people solved parts of it over time. I'm adding another part here
after I have discovered a crash related to it.

Even with this step, there are still problems remaining:
* `read_gitfile_gently()` still uses shared buffer.
* `absolute_path()` was not removed.

These issues remain because there're too many code references and I'd like
to avoid submitting a single topic of a scary size.

Alexandr Miloslavskiy (4):
  set_git_dir: fix crash when used with real_path()
  real_path: remove unsafe API
  real_path_if_valid(): remove unsafe API
  get_superproject_working_tree(): return strbuf

 abspath.c                  | 18 +-----------------
 builtin/clone.c            |  6 +++++-
 builtin/commit-graph.c     |  5 ++++-
 builtin/init-db.c          |  4 ++--
 builtin/rev-parse.c        | 12 ++++++++----
 builtin/worktree.c         |  9 ++++++---
 cache.h                    |  4 +---
 editor.c                   | 11 +++++++++--
 environment.c              | 18 ++++++++++++++++--
 path.c                     |  4 ++--
 setup.c                    | 35 ++++++++++++++++++++++-------------
 sha1-file.c                | 13 ++++---------
 submodule.c                | 22 ++++++++++++----------
 submodule.h                |  4 ++--
 t/helper/test-path-utils.c |  5 ++++-
 worktree.c                 | 12 +++++++++---
 16 files changed, 107 insertions(+), 75 deletions(-)

base-commit: 076cbdcd739aeb33c1be87b73aebae5e43d7bcc5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-575%2FSyntevoAlex%2F%230205(git)_crash_real_path-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-575/SyntevoAlex/#0205(git)_crash_real_path-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/575

Range-diff vs v1:

 1:  f7afcb4cc83 = 1:  f7afcb4cc83 set_git_dir: fix crash when used with real_path()
 2:  039d3d36866 ! 2:  29e7133dcd9 real_path: remove unsafe API
     @@ -64,7 +64,6 @@
       			die_errno(_("failed to unlink '%s'"), dest->buf);
       		if (!option_no_hardlinks) {
      -			if (!link(real_path(src->buf), dest->buf))
     -+			strbuf_reset(&realpath);
      +			strbuf_realpath(&realpath, src->buf, 1);
      +			if (!link(realpath.buf, dest->buf))
       				continue;
     @@ -92,7 +91,6 @@
       	prepare_alt_odb(r);
       	for (odb = r->objects->odb; odb; odb = odb->next) {
      -		if (!strcmp(obj_dir_real, real_path(odb->path)))
     -+		strbuf_reset(&odb_path_real);
      +		strbuf_realpath(&odb_path_real, odb->path, 1);
      +		if (!strcmp(obj_dir_real, odb_path_real.buf))
       			break;
     @@ -139,7 +137,6 @@
      -	write_file(sb.buf, "%s", real_path(sb_git.buf));
      +	strbuf_realpath(&realpath, sb_git.buf, 1);
      +	write_file(sb.buf, "%s", realpath.buf);
     -+	strbuf_reset(&realpath);
      +	strbuf_realpath(&realpath, get_git_common_dir(), 1);
       	write_file(sb_git.buf, "gitdir: %s/worktrees/%s",
      -		   real_path(get_git_common_dir()), name);
     @@ -261,7 +258,6 @@
       		if (*path == '/') {
       			*path = '\0';
      -			if (fspathcmp(real_path(path0), work_tree) == 0) {
     -+			strbuf_reset(&realpath);
      +			strbuf_realpath(&realpath, path0, 1);
      +			if (fspathcmp(realpath.buf, work_tree) == 0) {
       				memmove(path0, path + 1, len - (path - path0));
     @@ -274,7 +270,6 @@

       	/* check whole path */
      -	if (fspathcmp(real_path(path0), work_tree) == 0) {
     -+	strbuf_reset(&realpath);
      +	strbuf_realpath(&realpath, path0, 1);
      +	if (fspathcmp(realpath.buf, work_tree) == 0) {
       		*path0 = '\0';
     @@ -338,7 +333,6 @@
      +		struct strbuf realpath = STRBUF_INIT;
       		while (argc > 2) {
      -			puts(real_path(argv[2]));
     -+			strbuf_reset(&realpath);
      +			strbuf_realpath(&realpath, argv[2], 1);
      +			puts(realpath.buf);
       			argc--;
 3:  59af49ad9f6 ! 3:  a4917638671 real_path_if_valid(): remove unsafe API
     @@ -122,7 +122,6 @@
       		return NULL;
       	for (; *list; list++) {
      -		const char *wt_path = real_path_if_valid((*list)->path);
     -+		strbuf_reset(&wt_path);
      +		if (!strbuf_realpath(&wt_path, (*list)->path, 0))
      +			continue;

 4:  2eeefda3d41 ! 4:  41950069a16 get_superproject_working_tree(): return strbuf
     @@ -33,7 +33,7 @@
       }

      -const char *get_superproject_working_tree(void)
     -+int get_superproject_working_tree(struct strbuf* buf)
     ++int get_superproject_working_tree(struct strbuf *buf)
       {
      -	static struct strbuf realpath = STRBUF_INIT;
       	struct child_process cp = CHILD_PROCESS_INIT;
     @@ -94,6 +94,6 @@
      + * another repository, return 0.
        */
      -const char *get_superproject_working_tree(void);
     -+int get_superproject_working_tree(struct strbuf* buf);
     ++int get_superproject_working_tree(struct strbuf *buf);

       #endif

-- 
gitgitgadget