Re: [PATCH 06/19] untracked cache: record/validate dir mtime and reuse cached output

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, Oct 27, 2014 at 8:10 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> diff --git a/dir.c b/dir.c
> index 2793e57..55780a7 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -37,7 +37,12 @@ enum path_treatment {
> +static enum path_treatment treat_path_fast(struct dir_struct *dir,
> +                                          struct untracked_cache_dir *untracked,
> +                                          struct cached_dir *cdir,
> +                                          struct strbuf *path,
> +                                          int baselen,
> +                                          const struct path_simplify *simplify)
> +{
> +       if (!cdir->ucd) {
> +               strbuf_setlen(path, baselen);
> +               strbuf_addstr(path, cdir->file);
> +               return path_untracked;
> +       }
> +       strbuf_setlen(path, baseline);

Would it make sense to move the strbuf_setlen(path,baseline) above the
conditional since it is common to both cases, or are they conceptually
distinct enough that it is clearer to duplicate the function call for
each case?

> +       strbuf_addstr(path, cdir->ucd->name);
> +       /* treat_one_path() does this before it calls treat_directory() */
> +       if (path->buf[path->len - 1] != '/')
> +               strbuf_addch(path, '/');
> +       if (cdir->ucd->check_only)
> +               /*
> +                * check_only is set as a result of treat_directory() getting
> +                * to its bottom. Verify again the same set of directories
> +                * with check_only set.
> +                */
> +               return read_directory_recursive(dir, path->buf, path->len,
> +
> +                                               cdir->ucd, 1, simplify);

Unusual blank line placement.

> +       /*
> +        * We get path_recurse in the first run when
> +        * directory_exists_in_index() returns index_nonexistent. We
> +        * are sure that new changes in the index does not impact the
> +        * outcome. Return now.
> +        */
> +       return path_recurse;
> +}
> +
> @@ -1477,6 +1590,12 @@ static void close_cached_dir(struct cached_dir *cdir)
>  {
>         if (cdir->fdir)
>                 closedir(cdir->fdir);
> +       /*
> +        * We have gone through this directory and found no untracked
> +        * entries. Mark it valid.
> +        */
> +       if (cdir->untracked && !cdir->untracked->valid)
> +               cdir->untracked->valid = 1;

Or, stated more simply:

   if (cdir->untracked)
        cdir->untracked->valid = 1;

>  }
>
>  /*