Re: [PATCH 2/2] diff-lib: skip paths outside prefix in oneway_diff()

Jeff King <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <[email protected]>
On Sun, Jul 26, 2026 at 04:47:05AM -0400, Jeff King wrote:

> diff --git a/diff-lib.c b/diff-lib.c
> index 95f920a9a0..9986f5b141 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -528,6 +528,11 @@ static int oneway_diff(const struct cache_entry * const *src,
>  	if (tree == o->df_conflict_entry)
>  		tree = NULL;
>  
> +	if (revs->diffopt.prefix &&
> +	    strncmp((idx ? idx : tree)->name, revs->diffopt.prefix,
> +		    revs->diffopt.prefix_length))
> +		return 0;
> +

BTW, Coverity complains here that "tree" could be NULL (because we set
it that way in the lines above due to a D/F conflict).

I _think_ it is fine. We only look at "tree" if idx is NULL, and I think
idx is only NULL when we have a deletion. So that implies either:

  1. unpack_trees() passed us both entries as NULL, which doesn't make
     sense. There was no entry to delete!

  2. We set tree to NULL due to a D/F conflict. But a conflict with
     what? There is nothing at the path in the index to conflict.

So AFAICT this is OK and it's just a false positive from Coverity
(though an understandable one; the semantics of the relationship between
"idx" and "tree" are not represented in the code).

Possibly adding:

  if (!idx && !tree)
	BUG("oneway diff with no endpoints");

would help static analysis, but I don't know if that makes things more
or less clear to a human.

-Peff
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.