[4/4] get_superproject_working_tree(): return strbuf

Commit Message

Linus Arver via GitGitGadget March 6, 2020, 7:03 p.m. UTC

From: Alexandr Miloslavskiy <[e-mail]>

Together with the previous commits, this commit fully fixes the problem
of using shared buffer for `real_path()` in `get_superproject_working_tree()`.

Signed-off-by: Alexandr Miloslavskiy <[e-mail]>
---
 builtin/rev-parse.c |  7 ++++---
 submodule.c         | 17 ++++++++---------
 submodule.h         |  4 ++--
 3 files changed, 14 insertions(+), 14 deletions(-)

Comments

Junio C Hamano March 6, 2020, 10:44 p.m. UTC | #1

"Alexandr Miloslavskiy via GitGitGadget" <[e-mail]>
writes:

>  			if (!strcmp(arg, "--show-superproject-working-tree")) {
> -				const char *superproject = get_superproject_working_tree();
> -				if (superproject)
> -					puts(superproject);
> +				struct strbuf superproject = STRBUF_INIT;
> +				if (get_superproject_working_tree(&superproject))
> +					puts(superproject.buf);
> +				strbuf_release(&superproject);

The new calling convention makes sense here.

>  				continue;
>  			}
>  			if (!strcmp(arg, "--show-prefix")) {
> diff --git a/submodule.c b/submodule.c
> index 215c62580fc..46f6c2cbfd0 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -2168,14 +2168,13 @@ void absorb_git_dir_into_superproject(const char *path,
>  	}
>  }
>  
> -const char *get_superproject_working_tree(void)
> +int get_superproject_working_tree(struct strbuf* buf)

Micronit.  

The asterisk sticks to the identifier, not type, in our codebase.
I.e. "struct strbuf *buf".

> diff --git a/submodule.h b/submodule.h
> index c81ec1a9b6c..17492e478fc 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -152,8 +152,8 @@ void absorb_git_dir_into_superproject(const char *path,
>  /*
>   * Return the absolute path of the working tree of the superproject, which this
>   * project is a submodule of. If this repository is not a submodule of
> - * another repository, return NULL.
> + * another repository, return 0.
>   */
> -const char *get_superproject_working_tree(void);
> +int get_superproject_working_tree(struct strbuf* buf);

Likewise.

The conversion of the function body looked quite sensible.

Thanks.

Alexandr Miloslavskiy March 6, 2020, 11:06 p.m. UTC | #2

On 06.03.2020 23:44, Junio C Hamano wrote:
> Micronit.
> 
> The asterisk sticks to the identifier, not type, in our codebase.
> I.e. "struct strbuf *buf".

Sorry, having difficulties switching between many different styles.
Will fix in V2 next week.


Thank you very much for giving such a quick response! It is a great 
pleasure when my patches get movement instead of just gathering dust 
like in some other opensource projects :(

Patch

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 06ca7175ac7..06056434ed1 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -808,9 +808,10 @@  int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--show-superproject-working-tree")) {
-				const char *superproject = get_superproject_working_tree();
-				if (superproject)
-					puts(superproject);
+				struct strbuf superproject = STRBUF_INIT;
+				if (get_superproject_working_tree(&superproject))
+					puts(superproject.buf);
+				strbuf_release(&superproject);
 				continue;
 			}
 			if (!strcmp(arg, "--show-prefix")) {
diff --git a/submodule.c b/submodule.c
index 215c62580fc..46f6c2cbfd0 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2168,14 +2168,13 @@  void absorb_git_dir_into_superproject(const char *path,
 	}
 }
 
-const char *get_superproject_working_tree(void)
+int get_superproject_working_tree(struct strbuf* buf)
 {
-	static struct strbuf realpath = STRBUF_INIT;
 	struct child_process cp = CHILD_PROCESS_INIT;
 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf one_up = STRBUF_INIT;
 	const char *cwd = xgetcwd();
-	const char *ret = NULL;
+	int ret = 0;
 	const char *subpath;
 	int code;
 	ssize_t len;
@@ -2186,10 +2185,10 @@  const char *get_superproject_working_tree(void)
 		 * We might have a superproject, but it is harder
 		 * to determine.
 		 */
-		return NULL;
+		return 0;
 
 	if (!strbuf_realpath(&one_up, "../", 0))
-		return NULL;
+		return 0;
 
 	subpath = relative_path(cwd, one_up.buf, &sb);
 	strbuf_release(&one_up);
@@ -2233,8 +2232,8 @@  const char *get_superproject_working_tree(void)
 		super_wt = xstrdup(cwd);
 		super_wt[cwd_len - super_sub_len] = '\0';
 
-		strbuf_realpath(&realpath, super_wt, 1);
-		ret = realpath.buf;
+		strbuf_realpath(buf, super_wt, 1);
+		ret = 1;
 		free(super_wt);
 	}
 	strbuf_release(&sb);
@@ -2243,10 +2242,10 @@  const char *get_superproject_working_tree(void)
 
 	if (code == 128)
 		/* '../' is not a git repository */
-		return NULL;
+		return 0;
 	if (code == 0 && len == 0)
 		/* There is an unrelated git repository at '../' */
-		return NULL;
+		return 0;
 	if (code)
 		die(_("ls-tree returned unexpected return code %d"), code);
 
diff --git a/submodule.h b/submodule.h
index c81ec1a9b6c..17492e478fc 100644
--- a/submodule.h
+++ b/submodule.h
@@ -152,8 +152,8 @@  void absorb_git_dir_into_superproject(const char *path,
 /*
  * Return the absolute path of the working tree of the superproject, which this
  * project is a submodule of. If this repository is not a submodule of
- * another repository, return NULL.
+ * another repository, return 0.
  */
-const char *get_superproject_working_tree(void);
+int get_superproject_working_tree(struct strbuf* buf);
 
 #endif