Re: [PATCH v3 05/10] diff-lib: accept option flags in run_diff_index()

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

 



Denton Liu <liu.denton@xxxxxxxxx> writes:

> diff --git a/diff-lib.c b/diff-lib.c
> index 50521e2093..0a0e69113c 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -518,7 +518,7 @@ static int diff_cache(struct rev_info *revs,
>  	return unpack_trees(1, &t, &opts);
>  }
>  
> -int run_diff_index(struct rev_info *revs, int cached)
> +int run_diff_index(struct rev_info *revs, unsigned int option)
>  {
>  	struct object_array_entry *ent;

If we introduce

	int cached = !!(option & DIFF_INDEX_CACHED);

we do not have to touch the remainder of the function, and it makes
it easier to read the place(s) where 'cached' is used.  At that
point, the fact that we were instructed by a bit in the option flag
word is much much less important than we were instructed to compare
the tree-ish with the index and not the working tree files.

The same technique is used with DIFF_RACY_IS_MODIFIED flag in
run_diff_files().

> diff --git a/diff.h b/diff.h
> index e0c0af6286..0cc874f2d5 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -585,7 +585,8 @@ const char *diff_aligned_abbrev(const struct object_id *sha1, int);
>  /* report racily-clean paths as modified */
>  #define DIFF_RACY_IS_MODIFIED 02
>  int run_diff_files(struct rev_info *revs, unsigned int option);
> -int run_diff_index(struct rev_info *revs, int cached);
> +#define DIFF_INDEX_CACHED 01
> +int run_diff_index(struct rev_info *revs, unsigned int option);

It is unclear from the above if the option word for run_diff_files()
and run_diff_index() share the same bit assignment.  After reading
the series through to the end, I know they are independent set of
bits and never shared, but I wonder if we can make it more obvious
here.

Perhaps an extra blank line before this new #define is sufficient to
make it clear?



[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]

  Powered by Linux