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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 3, 2017, 20:17 UTC
Message-ID: <xmqqtw1owgn7.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170803182000.179328-3-bmwill@google.com>

Brandon Williams <bmwill@google.com> writes:

Show 24 quoted lines

> 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 <bmwill@google.com>
> ---
>  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.