Subject: Re: [PATCH v2 02/15] submodule: don't use submodule_from_name

From: Junio C Hamano <[email-masked]>
Date: Thu, 03 Aug 2017 13:17:00 -0700

Brandon Williams <[email-masked]> writes:

> The function 'submodule_from_name()' is being used incorrectly here as a
> submodule path is being used instead of a submodule name.  Since the
> correct function to use with a path to a submodule is already being used
> ('submodule_from_path()') let's remove the call to
> 'submodule_from_name()'.
>
> Signed-off-by: Brandon Williams <[email-masked]>
> ---
>  submodule.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 5139b9256..19bd13bb2 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1177,8 +1177,6 @@ static int get_next_submodule(struct child_process *cp,
>  			continue;
>  
>  		submodule = submodule_from_path(&null_oid, ce->name);
> -		if (!submodule)
> -			submodule = submodule_from_name(&null_oid, ce->name);
>  
>  		default_argv = "yes";
>  		if (spf->command_line_option == RECURSE_SUBMODULES_DEFAULT) {

It appears to me that the scope of the variable "submodule" in this
function can be narrowed to be limited to the block inside this "if"
statement we see in the post-context of this hunk.  That would make
it even easier to see why leaving submodule to NULL is a safe thing
to do.

This comment applies to the state of this function before or after
this patch.  It can be left outside the scope of this immediate
series, and instead be done as a follow-up (or preparatory) cleanup.

Thanks.