Re: [PATCH v3 11/26] submodule--helper: refactor "errmsg_str" to be a "struct strbuf"

> From: Glen Choo <chooglen@google.com>
>
> Refactor code added in e83e3333b57 (submodule: port submodule
> subcommand 'summary' from shell to C, 2020-08-13) so that "errmsg" and
> "errmsg_str" are folded into one. The distinction between the empty
> string and NULL is something that's tested for by
> e.g. "t/t7401-submodule-summary.sh".
>
> This is in preparation for fixing a memory leak the "struct strbuf" in
> the pre-image.
>
> Let's also pass a "const char *" to print_submodule_summary(), as it
> should not be modifying the "errmsg".
>
> Signed-off-by: Glen Choo <chooglen@google.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/submodule--helper.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 21b3abb7b40..f794d2b588b 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -866,7 +866,7 @@ static char *verify_submodule_committish(const char *sm_path,
>  	return strbuf_detach(&result, NULL);
>  }
>  
> -static void print_submodule_summary(struct summary_cb *info, char *errmsg,
> +static void print_submodule_summary(struct summary_cb *info, const char *errmsg,
>  				    int total_commits, const char *displaypath,
>  				    const char *src_abbrev, const char *dst_abbrev,
>  				    struct module_cb *p)
> @@ -924,7 +924,7 @@ static void generate_submodule_summary(struct summary_cb *info,
>  {
>  	char *displaypath, *src_abbrev = NULL, *dst_abbrev;
>  	int missing_src = 0, missing_dst = 0;
> -	char *errmsg = NULL;
> +	struct strbuf errmsg = STRBUF_INIT;
>  	int total_commits = -1;
>  
>  	if (!info->cached && oideq(&p->oid_dst, null_oid())) {
> @@ -1024,23 +1024,21 @@ static void generate_submodule_summary(struct summary_cb *info,
>  		 * submodule, i.e., deleted or changed to blob
>  		 */
>  		if (S_ISGITLINK(p->mod_dst)) {
> -			struct strbuf errmsg_str = STRBUF_INIT;
>  			if (missing_src && missing_dst) {
> -				strbuf_addf(&errmsg_str, "  Warn: %s doesn't contain commits %s and %s\n",
> +				strbuf_addf(&errmsg, "  Warn: %s doesn't contain commits %s and %s\n",
>  					    displaypath, oid_to_hex(&p->oid_src),
>  					    oid_to_hex(&p->oid_dst));
>  			} else {
> -				strbuf_addf(&errmsg_str, "  Warn: %s doesn't contain commit %s\n",
> +				strbuf_addf(&errmsg, "  Warn: %s doesn't contain commit %s\n",
>  					    displaypath, missing_src ?
>  					    oid_to_hex(&p->oid_src) :
>  					    oid_to_hex(&p->oid_dst));
>  			}
> -			errmsg = strbuf_detach(&errmsg_str, NULL);
>  		}
>  	}
>  
> -	print_submodule_summary(info, errmsg, total_commits,
> -				displaypath, src_abbrev,
> +	print_submodule_summary(info, errmsg.len ? errmsg.buf : NULL,
> +				total_commits, displaypath, src_abbrev,
>  				dst_abbrev, p);
>  
>  	free(displaypath);