Re: [PATCH v3 3/8] dir: recurse into untracked dirs for ignored files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Samuel Lijin <sxlijin@xxxxxxxxx> writes:

> We consider directories containing only untracked and ignored files to
> be themselves untracked, which in the usual case means we don't have to
> search these directories. This is problematic when we want to collect
> ignored files with DIR_SHOW_IGNORED_TOO, though, so we teach
> read_directory_recursive() to recurse into untracked directories to find
> the ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This
> has the side effect of also collecting all untracked files in untracked
> directories as well.
>
> Signed-off-by: Samuel Lijin <sxlijin@xxxxxxxxx>
> ---
>  dir.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/dir.c b/dir.c
> index f451bfa48..6bd0350e9 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1784,7 +1784,12 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,
>  			dir_state = state;
>  
>  		/* recurse into subdir if instructed by treat_path */
> -		if (state == path_recurse) {
> +		if ((state == path_recurse) ||
> +				((get_dtype(cdir.de, path.buf, path.len) == DT_DIR) &&

I see a conditional in treat_path() that is prepared to deal with a
NULL in cdir.de; do we know cdir.de is always non-NULL at this point
in the code, or is get_dtype() prepared to see NULL as its first
parameter?

	... goes and looks ...

Yes, it falls back to the usual lstat() dance in such a case, so
we'd be OK here.

> +				 (state == path_untracked) &&
> +				 (dir->flags & DIR_SHOW_IGNORED_TOO))

If we are told to SHOW_IGNORED_TOO, we'd recurse into an untracked
thing if it is a directory.  No other behaviour change.

Isn't get_dtype() potentially slower than other two checks if it
triggers?  I am wondering if these three conditions in &&-chain
should be reordered to call get_dtype() the last, i.e.

                if ((state == path_recurse) ||
                    ((dir->flags & DIR_SHOW_IGNORED_TOO)) &&
                     (state == path_untracked) &&
                     (get_dtype(cdir.de, path.buf, path.len) == DT_DIR)) {

> +				)
> +		{

It may be just a style, but these new lines are indented overly too
deep.  We don't use a lone "{" on a line to open a block, either.

>  			struct untracked_cache_dir *ud;
>  			ud = lookup_untracked(dir->untracked, untracked,
>  					      path.buf + baselen,



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]