On Wed, Feb 16, 2022 at 8:30 AM Elijah Newren [off-list ref] wrote:

On Wed, Feb 16, 2022 at 1:37 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

quoted

On Fri, Jan 14 2022, Elijah Newren via GitGitGadget wrote:

...

quoted

quoted

+
+     /* 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...