Re: [PATCH] ls-files: don't try to prune an empty index
- From
René Scharfe <l.s.r@web.de>
- Date
- Jul 16, 2017, 03:52 UTC
- Message-ID
- <ae5359cf-a480-f357-cb51-78f9d4cc431c@web.de>
- In-Reply-To
- <8fdcc21d-ccf9-7594-4d69-dfcad580da32@ramsayjones.plus.com>
Am 16.07.2017 um 02:28 schrieb Ramsay Jones:
Show 34 quoted lines
> > > 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 <l.s.r@web.de> >> --- >> 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é