Re: [PATCH v2 5/5] dir.c: stop ignoring opendir() error in open_cached_dir()

From: Duy Nguyen <pclouds@gmail.com>
Date: Jan 7, 2018, 12:44 UTC
Message-ID: <CACsJy8DG8H4r+-yAzo0Z8xuzb3FpDCayGaj0jqO4btkymrGN6Q@mail.gmail.com>
In-Reply-To: <20180103204928.3769-6-avarab@gmail.com>

On Thu, Jan 4, 2018 at 3:49 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 37 quoted lines

> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> A follow-up to the recently fixed bugs in the untracked
> invalidation. If opendir() fails it should show a warning, perhaps
> this should die, but if this ever happens the error is probably
> recoverable for the user, and dying would just make things worse.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  dir.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 163ca69df0..a605e01692 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1857,17 +1857,22 @@ static int open_cached_dir(struct cached_dir *cdir,
>                            struct strbuf *path,
>                            int check_only)
>  {
> +       const char *c_path;
> +
>         memset(cdir, 0, sizeof(*cdir));
>         cdir->untracked = untracked;
>         if (valid_cached_dir(dir, untracked, istate, path, check_only))
>                 return 0;
> -       cdir->fdir = opendir(path->len ? path->buf : ".");
> +       c_path = path->len ? path->buf : ".";
> +       cdir->fdir = opendir(c_path);
>         if (dir->untracked) {
>                 invalidate_directory(dir->untracked, untracked);
>                 dir->untracked->dir_opened++;
>         }
> -       if (!cdir->fdir)
> +       if (!cdir->fdir) {
> +               warning_errno(_("could not open directory '%s'"), c_path);

This should be closer to opendir(). The code in between, invalidate_directory(), could have modified errno and we would print incorrect message here.

But you can't simply move the whole "if (!cdir->fdir) { .. }" block up either because we want to invalidate before returning -1, so perhaps

       cdir->fdir = opendir(c_path);
       if (!cdir->fdir)
               warning_errno(_("could not open directory '%s'"), c_path);
       if (dir->untracked) { ... }
       if (!cdir->fdir)
               return -1;

Show 8 quoted lines

>                 return -1;
> +       }
>         return 0;
>  }
>
> --
> 2.15.1.424.g9478a66081
>

-- 
Duy