diff-lib: add idx/tree sanity check to oneway_diff

When looking just at the code in oneway_diff(), it seems possible for
both "idx" and "tree" to be NULL, in which case we'd potentially
segfault while checking the relative prefix.

But if you consider what these items actually mean, it shouldn't be
possible for both to be NULL. Let's add an assertion and a comment
documenting this. It might help human readers, but should also silence
static analyzers like Coverity which complain about the potential
segfault.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff-lib.c b/diff-lib.c
index a23119b852201270fe2b4e7e91cb726d1ed3249b..0e868b28b6b0042c0d2f04150426beefeb574414 100644 (file)
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -530,6 +530,16 @@ static int oneway_diff(const struct cache_entry * const *src,
        if (tree == o->df_conflict_entry)
                tree = NULL;
 
+       /*
+        * We should only see a NULL idx when the entry was present in the tree
+        * but deleted in the idx. In which case it should be impossible
+        * that a NULL tree was passed in (there would have been no entry at
+        * all) or that we got a df conflict above (you need a directory and a
+        * file to get such a conflict, which implies both sides are present).
+        */
+       if (!idx && !tree)
+               BUG("oneway_diff with neither idx nor tree");
+
        if (ce_path_match(revs->diffopt.repo->index,
                          idx ? idx : tree,
                          &revs->prune_data, NULL)) {