Re: [PATCH 4/4] get_superproject_working_tree(): return strbuf

From: Junio C Hamano <gitster@pobox.com>
To: "Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
	Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Subject: Re: [PATCH 4/4] get_superproject_working_tree(): return strbuf
Date: Fri, 06 Mar 2020 14:44:23 -0800	[thread overview]
Message-ID: <xmqqv9nh14u0.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <2eeefda3d41e6af1bc61249daf14b42050f0d0c3.1583521397.git.gitgitgadget@gmail.com> (Alexandr Miloslavskiy via GitGitGadget's message of "Fri, 06 Mar 2020 19:03:16 +0000")

"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
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.