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: > > > - 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. > > > Thank you for this entire patch series. Just one nit: > > > > > > > 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