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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Oct 23, 2019, 16:20 UTC
Message-ID: <20191023162048.GI4348@szeder.dev>
In-Reply-To: <xmqqa79si003.fsf@gitster-ct.c.googlers.com>

On Wed, Oct 23, 2019 at 01:01:00PM +0900, Junio C Hamano wrote:

Show 18 quoted lines

> SZEDER Gábor <szeder.dev@gmail.com> 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.