From 86ab7f0ccaed279271d562049c923a6264d2fc70 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 12 Aug 2011 23:43:04 +0200 Subject: [PATCH 1/7] Add a file comment Consolidate here a few general comments plus links to other documentation. Delete a comment with an out-of-date description of the .gitattributes file format. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- attr.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/attr.c b/attr.c index f6b3f7e850..6bc7ae9192 100644 --- a/attr.c +++ b/attr.c @@ -1,3 +1,12 @@ +/* + * Handle git attributes. See gitattributes(5) for a description of + * the file syntax, and Documentation/technical/api-gitattributes.txt + * for a description of the API. + * + * One basic design decision here is that we are not going to support + * an insanely large number of attributes. + */ + #define NO_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" #include "exec_cmd.h" @@ -13,12 +22,7 @@ static const char git_attr__unknown[] = "(builtin)unknown"; static const char *attributes_file; -/* - * The basic design decision here is that we are not going to have - * insanely large number of attributes. - * - * This is a randomly chosen prime. - */ +/* This is a randomly chosen prime. */ #define HASHSIZE 257 #ifndef DEBUG_ATTR @@ -103,16 +107,6 @@ struct git_attr *git_attr(const char *name) return git_attr_internal(name, strlen(name)); } -/* - * .gitattributes file is one line per record, each of which is - * - * (1) glob pattern. - * (2) whitespace - * (3) whitespace separated list of attribute names, each of which - * could be prefixed with '-' to mean "set to false", '!' to mean - * "unset". - */ - /* What does a matched pattern decide? */ struct attr_state { struct git_attr *attr; From ba845b755078a043312119609c1ddd7406b20979 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 12 Aug 2011 23:43:05 +0200 Subject: [PATCH 2/7] Document struct match_attr Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- attr.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/attr.c b/attr.c index 6bc7ae9192..c33e4135c3 100644 --- a/attr.c +++ b/attr.c @@ -113,6 +113,20 @@ struct attr_state { const char *setto; }; +/* + * One rule, as from a .gitattributes file. + * + * If is_macro is true, then u.attr is a pointer to the git_attr being + * defined. + * + * If is_macro is false, then u.pattern points at the filename pattern + * to which the rule applies. (The memory pointed to is part of the + * memory block allocated for the match_attr instance.) + * + * In either case, num_attr is the number of attributes affected by + * this rule, and state is an array listing them. The attributes are + * listed as they appear in the file (macros unexpanded). + */ struct match_attr { union { char *pattern; From 4c7517c9cc7cfc7961a5c31d7539a71f35fae2c2 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 12 Aug 2011 23:43:06 +0200 Subject: [PATCH 3/7] Increment num_attr in parse_attr_line(), not parse_attr() num_attr is incremented iff parse_attr() returns non-NULL. So do the counting in parse_attr_line() instead of within parse_attr(). This allows an integer rather than a pointer to an integer to be passed to parse_attr(). Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- attr.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/attr.c b/attr.c index c33e4135c3..cac550decd 100644 --- a/attr.c +++ b/attr.c @@ -140,7 +140,7 @@ struct match_attr { static const char blank[] = " \t\r\n"; static const char *parse_attr(const char *src, int lineno, const char *cp, - int *num_attr, struct match_attr *res) + int num_attr, struct match_attr *res) { const char *ep, *equals; int len; @@ -167,7 +167,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp, } else { struct attr_state *e; - e = &(res->state[*num_attr]); + e = &(res->state[num_attr]); if (*cp == '-' || *cp == '!') { e->setto = (*cp == '-') ? ATTR__FALSE : ATTR__UNSET; cp++; @@ -180,7 +180,6 @@ static const char *parse_attr(const char *src, int lineno, const char *cp, } e->attr = git_attr_internal(cp, len); } - (*num_attr)++; return ep + strspn(ep, blank); } @@ -226,9 +225,10 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, cp = name + namelen; cp = cp + strspn(cp, blank); while (*cp) { - cp = parse_attr(src, lineno, cp, &num_attr, res); + cp = parse_attr(src, lineno, cp, num_attr, res); if (!cp) return NULL; + num_attr++; } if (pass) break; From d175129857fe712bc7a4d882b0b9ef9f9d0a337e Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 12 Aug 2011 23:43:07 +0200 Subject: [PATCH 4/7] Change parse_attr() to take a pointer to struct attr_state parse_attr() only needs access to the attr_state to which it should store its results, not to the whole match_attr structure. This change also removes the need for it to know num_attr. Change its signature accordingly and add a comment. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- attr.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/attr.c b/attr.c index cac550decd..f23f62a6b1 100644 --- a/attr.c +++ b/attr.c @@ -139,8 +139,15 @@ struct match_attr { static const char blank[] = " \t\r\n"; +/* + * Parse a whitespace-delimited attribute state (i.e., "attr", + * "-attr", "!attr", or "attr=value") from the string starting at src. + * If e is not NULL, write the results to *e. Return a pointer to the + * remainder of the string (with leading whitespace removed), or NULL + * if there was an error. + */ static const char *parse_attr(const char *src, int lineno, const char *cp, - int num_attr, struct match_attr *res) + struct attr_state *e) { const char *ep, *equals; int len; @@ -153,7 +160,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp, len = equals - cp; else len = ep - cp; - if (!res) { + if (!e) { if (*cp == '-' || *cp == '!') { cp++; len--; @@ -165,9 +172,6 @@ static const char *parse_attr(const char *src, int lineno, const char *cp, return NULL; } } else { - struct attr_state *e; - - e = &(res->state[num_attr]); if (*cp == '-' || *cp == '!') { e->setto = (*cp == '-') ? ATTR__FALSE : ATTR__UNSET; cp++; @@ -225,7 +229,8 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, cp = name + namelen; cp = cp + strspn(cp, blank); while (*cp) { - cp = parse_attr(src, lineno, cp, num_attr, res); + cp = parse_attr(src, lineno, cp, + pass ? &(res->state[num_attr]) : NULL); if (!cp) return NULL; num_attr++; From 85c4a0d0482525480de4b575445ef8721858eff7 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 12 Aug 2011 23:43:08 +0200 Subject: [PATCH 5/7] Determine the start of the states outside of the pass loop Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- attr.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/attr.c b/attr.c index f23f62a6b1..a7d1aa95d2 100644 --- a/attr.c +++ b/attr.c @@ -192,7 +192,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, { int namelen; int num_attr; - const char *cp, *name; + const char *cp, *name, *states; struct match_attr *res = NULL; int pass; int is_macro; @@ -223,11 +223,13 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, else is_macro = 0; + states = name + namelen; + states += strspn(states, blank); + for (pass = 0; pass < 2; pass++) { /* pass 0 counts and allocates, pass 1 fills */ num_attr = 0; - cp = name + namelen; - cp = cp + strspn(cp, blank); + cp = states; while (*cp) { cp = parse_attr(src, lineno, cp, pass ? &(res->state[num_attr]) : NULL); From e0a5f9aaae4eea67f513e382d9d917388313d2a6 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 12 Aug 2011 23:43:09 +0200 Subject: [PATCH 6/7] Change while loop into for loop Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- attr.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/attr.c b/attr.c index a7d1aa95d2..b56542e723 100644 --- a/attr.c +++ b/attr.c @@ -228,14 +228,11 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, for (pass = 0; pass < 2; pass++) { /* pass 0 counts and allocates, pass 1 fills */ - num_attr = 0; - cp = states; - while (*cp) { + for (cp = states, num_attr = 0; *cp; num_attr++) { cp = parse_attr(src, lineno, cp, pass ? &(res->state[num_attr]) : NULL); if (!cp) return NULL; - num_attr++; } if (pass) break; From d68e1c183c056ae583a45b610d086ac078886154 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 12 Aug 2011 23:43:10 +0200 Subject: [PATCH 7/7] Unroll the loop over passes The passes no longer share much code, and the unrolled code is easier to understand. Use a new index variable instead of num_attr for the second loop, as we are no longer counting attributes but rather indexing through them. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- attr.c | 51 ++++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/attr.c b/attr.c index b56542e723..6abaaece32 100644 --- a/attr.c +++ b/attr.c @@ -191,10 +191,9 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, int lineno, int macro_ok) { int namelen; - int num_attr; + int num_attr, i; const char *cp, *name, *states; struct match_attr *res = NULL; - int pass; int is_macro; cp = line + strspn(line, blank); @@ -226,30 +225,32 @@ static struct match_attr *parse_attr_line(const char *line, const char *src, states = name + namelen; states += strspn(states, blank); - for (pass = 0; pass < 2; pass++) { - /* pass 0 counts and allocates, pass 1 fills */ - for (cp = states, num_attr = 0; *cp; num_attr++) { - cp = parse_attr(src, lineno, cp, - pass ? &(res->state[num_attr]) : NULL); - if (!cp) - return NULL; - } - if (pass) - break; - res = xcalloc(1, - sizeof(*res) + - sizeof(struct attr_state) * num_attr + - (is_macro ? 0 : namelen + 1)); - if (is_macro) - res->u.attr = git_attr_internal(name, namelen); - else { - res->u.pattern = (char *)&(res->state[num_attr]); - memcpy(res->u.pattern, name, namelen); - res->u.pattern[namelen] = 0; - } - res->is_macro = is_macro; - res->num_attr = num_attr; + /* First pass to count the attr_states */ + for (cp = states, num_attr = 0; *cp; num_attr++) { + cp = parse_attr(src, lineno, cp, NULL); + if (!cp) + return NULL; } + + res = xcalloc(1, + sizeof(*res) + + sizeof(struct attr_state) * num_attr + + (is_macro ? 0 : namelen + 1)); + if (is_macro) + res->u.attr = git_attr_internal(name, namelen); + else { + res->u.pattern = (char *)&(res->state[num_attr]); + memcpy(res->u.pattern, name, namelen); + res->u.pattern[namelen] = 0; + } + res->is_macro = is_macro; + res->num_attr = num_attr; + + /* Second pass to fill the attr_states */ + for (cp = states, i = 0; *cp; i++) { + cp = parse_attr(src, lineno, cp, &(res->state[i])); + } + return res; }