Re: [PATCH 1/4] attr.c: rename global var attr_nr to git_attr_nr
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 9, 2014, 23:54 UTC
- Message-ID
- <xmqq1to8l6qr.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1418133205-18213-2-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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"?
Show 63 quoted lines
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> 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;