Brandon Williams <[email protected]> writes: > +static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, > + int cached) > { > int hit = 0; > int nr; > + struct strbuf name = STRBUF_INIT; > + int name_base_len = 0; > + if (super_prefix) { > + name_base_len = strlen(super_prefix); > + strbuf_addstr(&name, super_prefix); > + } > + > read_cache(); > > for (nr = 0; nr < active_nr; nr++) { > const struct cache_entry *ce = active_cache[nr]; > - if (!S_ISREG(ce->ce_mode)) > - continue; > - if (!ce_path_match(ce, pathspec, NULL)) > - continue; > - /* > - * If CE_VALID is on, we assume worktree file and its cache > entry > - * are identical, even if worktree file has been modified, so > use > - * cache version instead > - */ > - if (cached || (ce->ce_flags & CE_VALID) || > ce_skip_worktree(ce)) { > - if (ce_stage(ce) || ce_intent_to_add(ce)) > - continue; > - hit |= grep_sha1(opt, ce->oid.hash, ce->name, 0, > - ce->name); > + strbuf_setlen(&name, name_base_len); > + strbuf_addstr(&name, ce->name); > + > + if (S_ISREG(ce->ce_mode) && > + match_pathspec(pathspec, name.buf, name.len, 0, NULL, > + S_ISDIR(ce->ce_mode) || > + S_ISGITLINK(ce->ce_mode))) { > + /* > + * If CE_VALID is on, we assume worktree file and its > + * cache entry are identical, even if worktree file has > + * been modified, so use cache version instead > + */ > + if (cached || (ce->ce_flags & CE_VALID) || > + ce_skip_worktree(ce)) { > + if (ce_stage(ce) || ce_intent_to_add(ce)) > + continue; > + hit |= grep_sha1(opt, ce->oid.hash, ce->name, > + 0, ce->name); > + } else { > + hit |= grep_file(opt, ce->name); > + } > + } else if (recurse_submodules && S_ISGITLINK(ce->ce_mode) && > + submodule_path_match(pathspec, name.buf, NULL)) { > + hit |= grep_submodule(opt, NULL, ce->name, ce->name); > } > - else > - hit |= grep_file(opt, ce->name);
We used to reject anything other than S_ISREG() upfront in the loop, and then either did grep_sha1() from the cache or from grep_file() from the working tree. Now, the guard upfront is removed, and we do the same in the first part of this if/elseif. The elseif part deals with a submodule that could match the pathspec. Don't we need a final else clause that would skip the remainder of this loop? What would happen to a S_ISREG() path that does *NOT* match the given pathspec? We used to just "continue", but it seems to me that such a path will fall through the above if/elseif in the new code. Would that be a problem?
