Re: [PATCH/RFC 4/4] attr: avoid heavy work when we know the specified attr is not defined

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

 



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

> +static void collect_selected_attrs(const char *path, int num,
> +				   struct git_attr_check *check)
> +{
> +	struct attr_stack *stk;
> +	int i, pathlen, rem, dirlen;
> +	int basename_offset;
> +
> +	pathlen = split_path(path, &dirlen, &basename_offset);
> +	prepare_attr_stack(path, dirlen);
> +	if (cannot_trust_maybe_real) {
> +		for (i = 0; i < git_attr_nr; i++)
> +			check_all_attr[i].value = ATTR__UNKNOWN;

Judging from the fact that

 (1) the only caller calls this function in this fashion based on the
     setting of "cannot-trust" bit,

 (2) this and the other function the only caller calls share the
     same code in their beginning part, and

 (3) the body of the if() statement here duplicates the code from
     collect_all_attrs(),

I smell that a much better split is possible.

Why isn't this all inside a single function collect_all_attrs()?
That single function may no longer be collect_ALL_attrs, so renaming
it to collect_attrs() is fine, but then that function may have this
if () to initialize all of them to ATTR__UNKNOWN or do the else part
we see below, and when organized that way we do not need to have
duplicated code (or split_path() helper function), no?

> +	} else {
> +		rem = num;
> +		for (i = 0; i < num; i++) {
> +			struct git_attr_check *c;
> +			c = check_all_attr + check[i].attr->attr_nr;
> +			if (check[i].attr->maybe_real)
> +				c->value = ATTR__UNKNOWN;
> +			else {
> +				c->value = ATTR__UNSET;
> +				rem--;
> +			}
> +		}
> +		if (!rem)
> +			return;
> +	}
> +	rem = git_attr_nr;
> +	for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
> +		rem = fill(path, pathlen, basename_offset, stk, rem);
> +}
> +
>  int git_check_attr(const char *path, int num, struct git_attr_check *check)
>  {
>  	int i;
>  
> -	collect_all_attrs(path);
> +	if (cannot_trust_maybe_real)
> +		collect_all_attrs(path);
> +	else
> +		collect_selected_attrs(path, num, check);
>  
>  	for (i = 0; i < num; i++) {
>  		const char *value = check_all_attr[check[i].attr->attr_nr].value;
--
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]