Re: [PATCH 5/5] attr.c: always pass check[] to collect_some_attrs()

[Stefan Beller <sbeller@google.com>]

On Wed, Jun 8, 2016 at 3:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> This function used to be called with check=NULL to signal it to
> collect all attributes in the global check_all_attr[] array.
>
> Because the longer term plan is to allocate check_all_attr[] and
> attr_stack data structures per git_attr_check instance (i.e. "check"
> here) to make the attr subsystem thread-safe, it is unacceptable.
>
> Pass "Are we grabbing all attributes defined in the system?" bit as
> a separate argument and pass it from the callers.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  attr.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index cdf064e..95d2f77 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -743,11 +743,12 @@ static int macroexpand_one(int nr, int rem)
>
>  /*
>   * Collect attributes for path into the array pointed to by
> - * check_all_attr.  If check is not NULL, only attributes in
> - * check[] are collected. Otherwise all attributes are collected.
> + * check_all_attr.  If collect_all is zero, only attributes in
> + * check[] are collected.  Otherwise, check[] is cleared and
> + * any and all attributes that are visible are collected in it.
>   */
>  static void collect_some_attrs(const char *path, int pathlen,
> -                              struct git_attr_check *check)
> +                              struct git_attr_check *check, int collect_all)

I think it may be better to have a collect_all_attrs instead of a flag here,
as more than half the executed code differs (the parts conditioned on
collect_all are rather large in the function)

>
>  {
>         struct attr_stack *stk;
> @@ -768,10 +769,11 @@ static void collect_some_attrs(const char *path, int pathlen,
>         }
>
>         prepare_attr_stack(path, dirlen);
> +

stray empty line?