Subject: Re: [PATCH v3 02/26] submodule--helper: stop conflating "sb" in clone_submodule()

From: Junio C Hamano <[email-masked]>
Date: Thu, 21 Jul 2022 14:16:36 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> -static int clone_submodule(struct module_clone_data *clone_data)
> +static char *clone_submodule_sm_gitdir(const char *name)
>  {
> -	char *p, *sm_gitdir;
> -	char *sm_alternate = NULL, *error_strategy = NULL;
>  	struct strbuf sb = STRBUF_INIT;
> -	struct child_process cp = CHILD_PROCESS_INIT;
> +	char *sm_gitdir;
>  
> -	submodule_name_to_gitdir(&sb, the_repository, clone_data->name);
> +	submodule_name_to_gitdir(&sb, the_repository, name);
>  	sm_gitdir = absolute_pathdup(sb.buf);
> -	strbuf_reset(&sb);
> +	strbuf_release(&sb);
> +
> +	return sm_gitdir;
> +}

OK.

> +static int clone_submodule(struct module_clone_data *clone_data)
> +{
> +	char *p;
> +	char *sm_gitdir = clone_submodule_sm_gitdir(clone_data->name);
> +	char *sm_alternate = NULL, *error_strategy = NULL;
> +	struct child_process cp = CHILD_PROCESS_INIT;
>  
>  	if (!is_absolute_path(clone_data->path)) {
> +		struct strbuf sb = STRBUF_INIT;
> +
>  		strbuf_addf(&sb, "%s/%s", get_git_work_tree(), clone_data->path);
>  		clone_data->path = strbuf_detach(&sb, NULL);

This looks like a roundabout way to say xstrfmt().

>  	} else {
> @@ -1705,6 +1715,8 @@ static int clone_submodule(struct module_clone_data *clone_data)
>  			die(_("clone of '%s' into submodule path '%s' failed"),
>  			    clone_data->url, clone_data->path);
>  	} else {
> +		struct strbuf sb = STRBUF_INIT;
> +
>  		if (clone_data->require_init && !access(clone_data->path, X_OK) &&
>  		    !is_empty_dir(clone_data->path))
>  			die(_("directory not empty: '%s'"), clone_data->path);
> @@ -1712,7 +1724,7 @@ static int clone_submodule(struct module_clone_data *clone_data)
>  			die(_("could not create directory '%s'"), clone_data->path);
>  		strbuf_addf(&sb, "%s/index", sm_gitdir);
>  		unlink_or_warn(sb.buf);
> -		strbuf_reset(&sb);
> +		strbuf_release(&sb);

Ditto.

>  	}
>  
>  	connect_work_tree_and_git_dir(clone_data->path, sm_gitdir, 0);
> @@ -1734,7 +1746,6 @@ static int clone_submodule(struct module_clone_data *clone_data)
>  	free(sm_alternate);
>  	free(error_strategy);
>  
> -	strbuf_release(&sb);
>  	free(sm_gitdir);
>  	free(p);
>  	return 0;