Re: [PATCH 1/4] attr.c: rename global var attr_nr to git_attr_nr

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

 



Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:

> This name "attr_nr" is used elsewhere as local variable, shadowing the
> global one. Let's rename the global one into something else to avoid
> accidents due to shadow in future.

Even though I do not think I would reject a patch to add this entire
file if the variable were named git_attr_nr from day one (read: the
result of the patch is not wrong per-se), I am not sure if the
variable deserves "git_" prefix that makes it look as if it may need
to be protected from global namespace contamination, given that this
is a file-scope static.

It might make a lot more sense not to do this rename, but change the
first parameter of macroexpand_one() from attr_nr to something more
appropriate, like "struct git_attr *attr"?

>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
>  attr.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index cd54697..583d36a 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -34,7 +34,7 @@ struct git_attr {
>  	int attr_nr;
>  	char name[FLEX_ARRAY];
>  };
> -static int attr_nr;
> +static int git_attr_nr;
>  
>  static struct git_attr_check *check_all_attr;
>  static struct git_attr *(git_attr_hash[HASHSIZE]);
> @@ -94,10 +94,10 @@ static struct git_attr *git_attr_internal(const char *name, int len)
>  	a->name[len] = 0;
>  	a->h = hval;
>  	a->next = git_attr_hash[pos];
> -	a->attr_nr = attr_nr++;
> +	a->attr_nr = git_attr_nr++;
>  	git_attr_hash[pos] = a;
>  
> -	REALLOC_ARRAY(check_all_attr, attr_nr);
> +	REALLOC_ARRAY(check_all_attr, git_attr_nr);
>  	check_all_attr[a->attr_nr].attr = a;
>  	check_all_attr[a->attr_nr].value = ATTR__UNKNOWN;
>  	return a;
> @@ -730,10 +730,10 @@ static void collect_all_attrs(const char *path)
>  	}
>  
>  	prepare_attr_stack(path, dirlen);
> -	for (i = 0; i < attr_nr; i++)
> +	for (i = 0; i < git_attr_nr; i++)
>  		check_all_attr[i].value = ATTR__UNKNOWN;
>  
> -	rem = attr_nr;
> +	rem = git_attr_nr;
>  	for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
>  		rem = fill(path, pathlen, basename_offset, stk, rem);
>  }
> @@ -762,7 +762,7 @@ int git_all_attrs(const char *path, int *num, struct git_attr_check **check)
>  
>  	/* Count the number of attributes that are set. */
>  	count = 0;
> -	for (i = 0; i < attr_nr; i++) {
> +	for (i = 0; i < git_attr_nr; i++) {
>  		const char *value = check_all_attr[i].value;
>  		if (value != ATTR__UNSET && value != ATTR__UNKNOWN)
>  			++count;
> @@ -770,7 +770,7 @@ int git_all_attrs(const char *path, int *num, struct git_attr_check **check)
>  	*num = count;
>  	*check = xmalloc(sizeof(**check) * count);
>  	j = 0;
> -	for (i = 0; i < attr_nr; i++) {
> +	for (i = 0; i < git_attr_nr; i++) {
>  		const char *value = check_all_attr[i].value;
>  		if (value != ATTR__UNSET && value != ATTR__UNKNOWN) {
>  			(*check)[j].attr = check_all_attr[i].attr;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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