Re: [PATCH] merge-recursive: use fspathcmp() in path_hashmap_cmp()

On Mon, Aug 30, 2021 at 05:09:34PM +0200, René Scharfe wrote:

quoted

diff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c
index 36ff07bd4b..ab34bdfecd 100644
--- a/t/helper/test-hashmap.c
+++ b/t/helper/test-hashmap.c

@@ -28,10 +28,7 @@ static int test_entry_cmp(const void *cmp_data,
        e1 = container_of(eptr, const struct test_entry, ent);
        e2 = container_of(entry_or_key, const struct test_entry, ent);

-       if (ignore_case)
-               return strcasecmp(e1->key, key ? key : e2->key);
-       else
-               return strcmp(e1->key, key ? key : e2->key);
+       return fspathcmp(e1->key, key ? key : e2->key);
 }

 static struct test_entry *alloc_test_entry(unsigned int hash,

That's a local variable named "ignore_case", not the one declared in
environment.c that fspathcmp() uses, so this would change the behavior.
The helper code does not include cache.h, so this is not even a case of
variable shadowing, just two different variables for similar purposes
in different places having the same name.

Yikes, good catch. Perhaps it's overkill, but I wonder if a comment
like:

  /*
   * Do not use fspathcmp() here; our behavior depends on the local
   * ignore_case variable, not the usual Git-wide global.
   */

would help.

I double-checked the spot I suggested. I think it is actually using the
global (though I got it right through sheer luck).

-Peff