Hi Gábor,

On Mon, 28 Oct 2019, SZEDER Gábor wrote:

On Mon, Oct 28, 2019 at 11:57:10AM +0100, Johannes Schindelin wrote:

quoted

quoted

  - According to the comment describing trie_find(), it should only
    call the given match function 'fn' for a "/-or-\0-terminated
    prefix of the key for which the trie contains a value".  This is
    not true: there are three places where trie_find() calls the match
    function, but one of them is missing the check for value's
    existence.

quoted

Thank you for this entire patch series. Just one nit:

quoted

diff --git a/path.c b/path.c
index cf57bd52dd..e21b00c4d4 100644
--- a/path.c
+++ b/path.c

@@ -299,9 +299,13 @@ static int trie_find(struct trie *root, const char *key, match_fn fn,

 	/* Matched the entire compressed section */
 	key += i;
-	if (!*key)
+	if (!*key) {
 		/* End of key */
-		return fn(key, root->value, baton);
+		if (root->value)
+			return fn(key, root->value, baton);
+		else
+			return -1;

I would have preferred this:

+		if (!root->value)
+			return -1;
+		return fn(key, root->value, baton);

... as it would more accurately reflect my mental model of an "early
out".

The checks at the other two of those three callsites look like this,
and I just followed suit for the sake of consistency.

Oh, okay. Sorry for the noise, then.

Thanks,
Dscho