Re: [PATCH v2 5/5] Accelerate clear_skip_worktree_from_present_files() by caching

[Elijah Newren <newren@gmail.com>]

On Wed, Feb 16, 2022 at 8:30 AM Elijah Newren <newren@gmail.com> wrote:
>
> On Wed, Feb 16, 2022 at 1:37 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> >
> > On Fri, Jan 14 2022, Elijah Newren via GitGitGadget wrote:
> >
...
> > > +
> > > +     /* Free previous dirname, and cache path's dirname */
> > > +     *dirname = path;
> > > +     *dir_len = newdir - path + 1;
> > > +
> > > +     tmp = xstrndup(path, *dir_len);
> > > +     *dir_found = !lstat(tmp, &st);
> >
> > In most other places we're a bit more careful about lstat() error handling, e.g.:
> >
> >     builtin/init-db.c:              if (lstat(path->buf, &st_git)) {
> >     builtin/init-db.c-                      if (errno != ENOENT)
> >     builtin/init-db.c-                              die_errno(_("cannot stat '%s'"), path->buf);
> >     builtin/init-db.c-              }
> >
> > Shouldn't we do the same here and at least error() on return values of
> > -1 with an accompanying errno that isn't ENOENT?
>
> If we should do that everywhere, should we have an xlstat in wrapper.[ch]?

Turns out we already DO have a file_exists() wrapper function, but
it's found in dir.c rather than wrapper.[ch].  However, it doesn't
bother to check errno either...