On Wed, Jun 8, 2016 at 3:58 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> The callchain that starts from git_check_attrs() down to
> collect_some_attrs() used to take an array of git_attr_check_elem
> as their parameters. Pass the enclosing git_attr_check instance
> instead, so that they will have access to new fields we will add to
> the data structure.
>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> attr.c | 36 ++++++++++++++++++++++++------------
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index 26228ce..4e2172a 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -746,14 +746,25 @@ static int macroexpand_one(int nr, int rem)
> * check_all_attr. If num is non-zero, only attributes in check[] are
> * collected. Otherwise all attributes are collected.
> */
> -static void collect_some_attrs(const char *path, int pathlen, int num,
> - struct git_attr_check_elem *check)
> +static void collect_some_attrs(const char *path, int pathlen,
> + struct git_attr_check *check)
>
> {
> struct attr_stack *stk;
> int i, rem, dirlen;
> const char *cp, *last_slash = NULL;
> int basename_offset;
> + int num;
> + struct git_attr_check_elem *celem;
> +
> + if (!check) {
> + /* Yuck - ugly git_all_attrs() hack! */
> + celem = NULL;
> + num = 0;
> + } else {
> + celem = check->check;
> + num = check->check_nr;
> + }
>
> for (cp = path; cp < path + pathlen; cp++) {
> if (*cp == '/' && cp[1])
> @@ -773,9 +784,9 @@ static void collect_some_attrs(const char *path, int pathlen, int num,
> if (num && !cannot_trust_maybe_real) {
> rem = 0;
> for (i = 0; i < num; i++) {
> - if (!check[i].attr->maybe_real) {
> + if (!celem[i].attr->maybe_real) {
> struct git_attr_check_elem *c;
> - c = check_all_attr + check[i].attr->attr_nr;
> + c = check_all_attr + celem[i].attr->attr_nr;
> c->value = ATTR__UNSET;
> rem++;
> }
> @@ -789,18 +800,19 @@ static void collect_some_attrs(const char *path, int pathlen, int num,
> rem = fill(path, pathlen, basename_offset, stk, rem);
> }
>
> -static int git_check_attrs(const char *path, int pathlen, int num,
> - struct git_attr_check_elem *check)
> +static int git_check_attrs(const char *path, int pathlen,
> + struct git_attr_check *check)
> {
> int i;
> + struct git_attr_check_elem *celem = check->check;
We don't need this outside the for loop; maybe we want to move
this in there including indexing?
Or rather dropping the arrays and use pointers
in there if we're concerned about performance?
(That would be a slightly larger refactoring, roughly):
struct git_attr_check_elem *cur = check->check;
struct git_attr_check_elem *end = cur + check->check_nr;
while (cur < end) {
value = check_all_attr[cur->attr->attr_nr].value;
cur->value = (value == ATTR__UNKNOWN) ? ATTR__UNSET : value;
cur++;
}
Looks correct as it is, though.
Thanks,
Stefan
--
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