Subject: Re: [PATCH 5/5] path.c: don't call the match function without value in trie_find()

From: SZEDER Gábor <[email-masked]>
Date: Wed, 23 Oct 2019 18:20:48 +0200

On Wed, Oct 23, 2019 at 01:01:00PM +0900, Junio C Hamano wrote:
> SZEDER Gábor <[email-masked]> writes:
> 
> >>   - b9317d55a3 added two new keys to the trie: 'logs/refs/rewritten'
> >>     and 'logs/refs/worktree', next to the already existing
> >>     'logs/refs/bisect'.  This resulted in a trie node with the path
> >>     'logs/refs', which didn't exist before, and which doesn't have a
> >
> > Oops, I missed the trailing slash, that must be 'logs/refs/'!
> >
> >>     value attached.  A query for 'logs/refs/' finds this node and then
> >>     hits that one callsite of the match function which doesn't check
> >>     for the value's existence, and thus invokes the match function
> >>     with NULL as value.
> 
> Given that the trie is maintained by hand in common_list[], I wonder
> if we can mechanically catch errors like the one b9317d55a3 added,
> by perhaps having a self-test function that a t/helper/ program
> calls to perform consistency check after the "git" gets built.

I'm not sure what you mean by "consistency check".  The resulting trie
looked as expected both before and after b9317d55a3, i.e. each trie
node had the right contents, value, and children, as far as I could
tell.  The issue was in the lookup function.