From: René Scharfe Date: Sun, 16 Jul 2017 03:52:42 GMT Subject: Re: [PATCH] ls-files: don't try to prune an empty index Message-ID: In-Reply-To: <8fdcc21d-ccf9-7594-4d69-dfcad580da32@ramsayjones.plus.com> Am 16.07.2017 um 02:28 schrieb Ramsay Jones: > > > On 15/07/17 21:11, René Scharfe wrote: >> Exit early when asked to prune an index that contains no >> entries to begin with. This avoids pointer arithmetic on >> istate->cache, which is possibly NULL in that case. >> >> Found with Clang's UBSan. >> >> Signed-off-by: Rene Scharfe >> --- >> builtin/ls-files.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/builtin/ls-files.c b/builtin/ls-files.c >> index b8514a0029..adf572da68 100644 >> --- a/builtin/ls-files.c >> +++ b/builtin/ls-files.c >> @@ -362,7 +362,7 @@ static void prune_index(struct index_state *istate, >> int pos; >> unsigned int first, last; >> >> - if (!prefix) >> + if (!prefix || !istate->cache_nr) >> return; >> pos = index_name_pos(istate, prefix, prefixlen); >> if (pos < 0) > > My patch looked like: > > - if (!prefix) > + if (!prefix || !istate->cache || istate->cache_nr == 0) > > ... which is probably a bit 'belt-n-braces'. ;-) Not checking for !istate->cache at this point is a good thing, I think. If we have entries, then ->cache must not be NULL, and if it is we'd get a segfault, notifying us that we have a bug. We could add an assert to state this requirement explicitly, but that would be the topic of a different patch. René