Skip to content

Commit 685b292

Browse files
bmwillgitster
bmwill
authored and
gitster
committed
attr: eliminate global check_all_attr array
Currently there is a reliance on 'check_all_attr' which is a global array of 'attr_check_item' items which is used to store the value of each attribute during the collection process. This patch eliminates this global and instead creates an array per 'attr_check' instance which is then used in the attribute collection process. This brings the attribute system one step closer to being thread-safe. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 1a600b7 commit 685b292

File tree

2 files changed

+87
-39
lines changed

2 files changed

+87
-39
lines changed

‎attr.c‎

Lines changed: 82 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ struct git_attr {
3434
int maybe_real;
3535
char name[FLEX_ARRAY]; /* attribute name */
3636
};
37-
static int attr_nr;
3837

3938
/*
4039
* NEEDSWORK: maybe-real, maybe-macro are not property of
@@ -45,9 +44,6 @@ static int attr_nr;
4544
*/
4645
static int cannot_trust_maybe_real;
4746

48-
/* NEEDSWORK: This will become per git_attr_check */
49-
static struct attr_check_item *check_all_attr;
50-
5147
const char *git_attr_name(const struct git_attr *attr)
5248
{
5349
return attr->name;
@@ -143,6 +139,57 @@ static void attr_hashmap_add(struct attr_hashmap *map,
143139
hashmap_add(&map->map, e);
144140
}
145141

142+
struct all_attrs_item {
143+
const struct git_attr *attr;
144+
const char *value;
145+
};
146+
147+
/*
148+
* Reallocate and reinitialize the array of all attributes (which is used in
149+
* the attribute collection process) in 'check' based on the global dictionary
150+
* of attributes.
151+
*/
152+
static void all_attrs_init(struct attr_hashmap *map, struct attr_check *check)
153+
{
154+
int i;
155+
156+
hashmap_lock(map);
157+
158+
if (map->map.size < check->all_attrs_nr)
159+
die("BUG: interned attributes shouldn't be deleted");
160+
161+
/*
162+
* If the number of attributes in the global dictionary has increased
163+
* (or this attr_check instance doesn't have an initialized all_attrs
164+
* field), reallocate the provided attr_check instance's all_attrs
165+
* field and fill each entry with its corresponding git_attr.
166+
*/
167+
if (map->map.size != check->all_attrs_nr) {
168+
struct attr_hash_entry *e;
169+
struct hashmap_iter iter;
170+
hashmap_iter_init(&map->map, &iter);
171+
172+
REALLOC_ARRAY(check->all_attrs, map->map.size);
173+
check->all_attrs_nr = map->map.size;
174+
175+
while ((e = hashmap_iter_next(&iter))) {
176+
const struct git_attr *a = e->value;
177+
check->all_attrs[a->attr_nr].attr = a;
178+
}
179+
}
180+
181+
hashmap_unlock(map);
182+
183+
/*
184+
* Re-initialize every entry in check->all_attrs.
185+
* This re-initialization can live outside of the locked region since
186+
* the attribute dictionary is no longer being accessed.
187+
*/
188+
for (i = 0; i < check->all_attrs_nr; i++) {
189+
check->all_attrs[i].value = ATTR__UNKNOWN;
190+
}
191+
}
192+
146193
static int attr_name_valid(const char *name, size_t namelen)
147194
{
148195
/*
@@ -196,16 +243,6 @@ static struct git_attr *git_attr_internal(const char *name, int namelen)
196243

197244
attr_hashmap_add(&g_attr_hashmap, a->name, namelen, a);
198245
assert(a->attr_nr == (g_attr_hashmap.map.size - 1));
199-
200-
/*
201-
* NEEDSWORK: per git_attr_check check_all_attr
202-
* will be initialized a lot more lazily, not
203-
* like this, and not here.
204-
*/
205-
REALLOC_ARRAY(check_all_attr, ++attr_nr);
206-
check_all_attr[a->attr_nr].attr = a;
207-
check_all_attr[a->attr_nr].value = ATTR__UNKNOWN;
208-
assert(a->attr_nr == (attr_nr - 1));
209246
}
210247

211248
hashmap_unlock(&g_attr_hashmap);
@@ -513,6 +550,10 @@ void attr_check_clear(struct attr_check *check)
513550
check->items = NULL;
514551
check->alloc = 0;
515552
check->nr = 0;
553+
554+
free(check->all_attrs);
555+
check->all_attrs = NULL;
556+
check->all_attrs_nr = 0;
516557
}
517558

518559
void attr_check_free(struct attr_check *check)
@@ -860,16 +901,16 @@ static int path_matches(const char *pathname, int pathlen,
860901
pattern, prefix, pat->patternlen, pat->flags);
861902
}
862903

863-
static int macroexpand_one(int attr_nr, int rem);
904+
static int macroexpand_one(struct all_attrs_item *all_attrs, int nr, int rem);
864905

865-
static int fill_one(const char *what, struct match_attr *a, int rem)
906+
static int fill_one(const char *what, struct all_attrs_item *all_attrs,
907+
struct match_attr *a, int rem)
866908
{
867-
struct attr_check_item *check = check_all_attr;
868909
int i;
869910

870-
for (i = a->num_attr - 1; 0 < rem && 0 <= i; i--) {
911+
for (i = a->num_attr - 1; rem > 0 && i >= 0; i--) {
871912
struct git_attr *attr = a->state[i].attr;
872-
const char **n = &(check[attr->attr_nr].value);
913+
const char **n = &(all_attrs[attr->attr_nr].value);
873914
const char *v = a->state[i].setto;
874915

875916
if (*n == ATTR__UNKNOWN) {
@@ -878,14 +919,15 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
878919
attr, v);
879920
*n = v;
880921
rem--;
881-
rem = macroexpand_one(attr->attr_nr, rem);
922+
rem = macroexpand_one(all_attrs, attr->attr_nr, rem);
882923
}
883924
}
884925
return rem;
885926
}
886927

887928
static int fill(const char *path, int pathlen, int basename_offset,
888-
struct attr_stack *stk, int rem)
929+
struct attr_stack *stk, struct all_attrs_item *all_attrs,
930+
int rem)
889931
{
890932
int i;
891933
const char *base = stk->origin ? stk->origin : "";
@@ -896,18 +938,18 @@ static int fill(const char *path, int pathlen, int basename_offset,
896938
continue;
897939
if (path_matches(path, pathlen, basename_offset,
898940
&a->u.pat, base, stk->originlen))
899-
rem = fill_one("fill", a, rem);
941+
rem = fill_one("fill", all_attrs, a, rem);
900942
}
901943
return rem;
902944
}
903945

904-
static int macroexpand_one(int nr, int rem)
946+
static int macroexpand_one(struct all_attrs_item *all_attrs, int nr, int rem)
905947
{
906948
struct attr_stack *stk;
907949
int i;
908950

909-
if (check_all_attr[nr].value != ATTR__TRUE ||
910-
!check_all_attr[nr].attr->maybe_macro)
951+
if (all_attrs[nr].value != ATTR__TRUE ||
952+
!all_attrs[nr].attr->maybe_macro)
911953
return rem;
912954

913955
for (stk = attr_stack; stk; stk = stk->prev) {
@@ -916,17 +958,17 @@ static int macroexpand_one(int nr, int rem)
916958
if (!ma->is_macro)
917959
continue;
918960
if (ma->u.attr->attr_nr == nr)
919-
return fill_one("expand", ma, rem);
961+
return fill_one("expand", all_attrs, ma, rem);
920962
}
921963
}
922964

923965
return rem;
924966
}
925967

926968
/*
927-
* Collect attributes for path into the array pointed to by
928-
* check_all_attr. If num is non-zero, only attributes in check[] are
929-
* collected. Otherwise all attributes are collected.
969+
* Collect attributes for path into the array pointed to by check->all_attrs.
970+
* If check->check_nr is non-zero, only attributes in check[] are collected.
971+
* Otherwise all attributes are collected.
930972
*/
931973
static void collect_some_attrs(const char *path, struct attr_check *check)
932974
{
@@ -949,15 +991,15 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
949991
}
950992

951993
prepare_attr_stack(path, dirlen);
952-
for (i = 0; i < attr_nr; i++)
953-
check_all_attr[i].value = ATTR__UNKNOWN;
994+
all_attrs_init(&g_attr_hashmap, check);
995+
954996
if (check->nr && !cannot_trust_maybe_real) {
955997
rem = 0;
956998
for (i = 0; i < check->nr; i++) {
957999
const struct git_attr *a = check->items[i].attr;
9581000
if (!a->maybe_real) {
959-
struct attr_check_item *c;
960-
c = check_all_attr + a->attr_nr;
1001+
struct all_attrs_item *c;
1002+
c = check->all_attrs + a->attr_nr;
9611003
c->value = ATTR__UNSET;
9621004
rem++;
9631005
}
@@ -966,9 +1008,9 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
9661008
return;
9671009
}
9681010

969-
rem = attr_nr;
1011+
rem = check->all_attrs_nr;
9701012
for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
971-
rem = fill(path, pathlen, basename_offset, stk, rem);
1013+
rem = fill(path, pathlen, basename_offset, stk, check->all_attrs, rem);
9721014
}
9731015

9741016
int git_check_attr(const char *path, struct attr_check *check)
@@ -978,7 +1020,8 @@ int git_check_attr(const char *path, struct attr_check *check)
9781020
collect_some_attrs(path, check);
9791021

9801022
for (i = 0; i < check->nr; i++) {
981-
const char *value = check_all_attr[check->items[i].attr->attr_nr].value;
1023+
size_t n = check->items[i].attr->attr_nr;
1024+
const char *value = check->all_attrs[n].value;
9821025
if (value == ATTR__UNKNOWN)
9831026
value = ATTR__UNSET;
9841027
check->items[i].value = value;
@@ -994,9 +1037,9 @@ void git_all_attrs(const char *path, struct attr_check *check)
9941037
attr_check_reset(check);
9951038
collect_some_attrs(path, check);
9961039

997-
for (i = 0; i < attr_nr; i++) {
998-
const char *name = check_all_attr[i].attr->name;
999-
const char *value = check_all_attr[i].value;
1040+
for (i = 0; i < check->all_attrs_nr; i++) {
1041+
const char *name = check->all_attrs[i].attr->name;
1042+
const char *value = check->all_attrs[i].value;
10001043
struct attr_check_item *item;
10011044
if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
10021045
continue;

‎attr.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
/* An attribute is a pointer to this opaque structure */
55
struct git_attr;
66

7+
/* opaque structure used internally for attribute collection */
8+
struct all_attrs_item;
9+
710
/*
811
* Given a string, return the gitattribute object that
912
* corresponds to it.
@@ -33,6 +36,8 @@ struct attr_check {
3336
int nr;
3437
int alloc;
3538
struct attr_check_item *items;
39+
int all_attrs_nr;
40+
struct all_attrs_item *all_attrs;
3641
};
3742

3843
extern struct attr_check *attr_check_alloc(void);

0 commit comments

Comments
 (0)