From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 09 Dec 2016 00:28:56 GMT
Subject: Re: [PATCH v2 14/16] pathspec: create strip submodule slash helpers
Message-ID: <xmqqfulyrlmf.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1481223550-65277-15-git-send-email-bmwill@google.com>

Brandon Williams <bmwill@google.com> writes:

> +static void strip_submodule_slash_cheap(struct pathspec_item *item)
> +{
> +	int i;
> +
> +	if ((item->len >= 1 && item->match[item->len - 1] == '/') &&
> +	    (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
> +	    S_ISGITLINK(active_cache[i]->ce_mode)) {
> +		item->len--;
> +		item->match[item->len] = '\0';
> +	}
> +}

I know that this is merely a moved code, but while I was reading
this, it triggered "Do not make an assignment inside if condition"
check.  But more importantly, is the code even correct?  If the path
for the submodule is unmerged, we would get a negative i that points
at the conflicting entry; don't we want to do something about it, at
least when we have a submodule entry at stage #2 (i.e. ours)?