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>
Brandon Williams committed
Jan 27, 2017 at 18:02 UTC
685b2925757e98624e37abe09d49c205a7db5943
2 files changed
+87
-39
attr.c
+82
-39
@@ -34,7 +34,6 @@ struct git_attr {
34
int maybe_real;
35
char name[FLEX_ARRAY]; /* attribute name */
36
};
37
-static int attr_nr;
37
38
/*
39
* NEEDSWORK: maybe-real, maybe-macro are not property of
@@ -45,9 +44,6 @@ static int attr_nr;
44
*/
45
static int cannot_trust_maybe_real;
46
48
-/* NEEDSWORK: This will become per git_attr_check */
49
-static struct attr_check_item *check_all_attr;
50
-
47
const char *git_attr_name(const struct git_attr *attr)
48
{
49
return attr->name;
@@ -143,6 +139,57 @@ static void attr_hashmap_add(struct attr_hashmap *map,
139
hashmap_add(&map->map, e);
140
}
141
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
+
193
static int attr_name_valid(const char *name, size_t namelen)
194
{
195
/*
@@ -196,16 +243,6 @@ static struct git_attr *git_attr_internal(const char *name, int namelen)
243
244
attr_hashmap_add(&g_attr_hashmap, a->name, namelen, a);
245
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));
246
}
247
248
hashmap_unlock(&g_attr_hashmap);
@@ -513,6 +550,10 @@ void attr_check_clear(struct attr_check *check)
550
check->items = NULL;
551
check->alloc = 0;
552
check->nr = 0;
553
+
554
+ free(check->all_attrs);
555
+ check->all_attrs = NULL;
556
+ check->all_attrs_nr = 0;
557
}
558
559
void attr_check_free(struct attr_check *check)
@@ -860,16 +901,16 @@ static int path_matches(const char *pathname, int pathlen,
901
pattern, prefix, pat->patternlen, pat->flags);
902
}
903
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);
905
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)
908
{
867
- struct attr_check_item *check = check_all_attr;
909
int i;
910
870
- for (i = a->num_attr - 1; 0 < rem && 0 <= i; i--) {
911
+ for (i = a->num_attr - 1; rem > 0 && i >= 0; i--) {
912
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);
914
const char *v = a->state[i].setto;
915
916
if (*n == ATTR__UNKNOWN) {
@@ -878,14 +919,15 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
919
attr, v);
920
*n = v;
921
rem--;
881
- rem = macroexpand_one(attr->attr_nr, rem);
922
+ rem = macroexpand_one(all_attrs, attr->attr_nr, rem);
923
}
924
}
925
return rem;
926
}
927
928
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)
931
{
932
int i;
933
const char *base = stk->origin ? stk->origin : "";
@@ -896,18 +938,18 @@ static int fill(const char *path, int pathlen, int basename_offset,
938
continue;
939
if (path_matches(path, pathlen, basename_offset,
940
&a->u.pat, base, stk->originlen))
899
- rem = fill_one("fill", a, rem);
941
+ rem = fill_one("fill", all_attrs, a, rem);
942
}
943
return rem;
944
}
945
904
-static int macroexpand_one(int nr, int rem)
946
+static int macroexpand_one(struct all_attrs_item *all_attrs, int nr, int rem)
947
{
948
struct attr_stack *stk;
949
int i;
950
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)
953
return rem;
954
955
for (stk = attr_stack; stk; stk = stk->prev) {
@@ -916,7 +958,7 @@ static int macroexpand_one(int nr, int rem)
958
if (!ma->is_macro)
959
continue;
960
if (ma->u.attr->attr_nr == nr)
919
- return fill_one("expand", ma, rem);
961
+ return fill_one("expand", all_attrs, ma, rem);
962
}
963
}
964
@@ -924,9 +966,9 @@ static int macroexpand_one(int nr, int rem)
966
}
967
968
/*
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.
972
*/
973
static void collect_some_attrs(const char *path, struct attr_check *check)
974
{
@@ -949,15 +991,15 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
991
}
992
993
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
+
996
if (check->nr && !cannot_trust_maybe_real) {
997
rem = 0;
998
for (i = 0; i < check->nr; i++) {
999
const struct git_attr *a = check->items[i].attr;
1000
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;
1003
c->value = ATTR__UNSET;
1004
rem++;
1005
}
@@ -966,9 +1008,9 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
1008
return;
1009
}
1010
969
- rem = attr_nr;
1011
+ rem = check->all_attrs_nr;
1012
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);
1014
}
1015
1016
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)
1020
collect_some_attrs(path, check);
1021
1022
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;
1025
if (value == ATTR__UNKNOWN)
1026
value = ATTR__UNSET;
1027
check->items[i].value = value;
@@ -994,9 +1037,9 @@ void git_all_attrs(const char *path, struct attr_check *check)
1037
attr_check_reset(check);
1038
collect_some_attrs(path, check);
1039
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;
1043
struct attr_check_item *item;
1044
if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
1045
continue;
attr.h
+5
@@ -4,6 +4,9 @@
4
/* An attribute is a pointer to this opaque structure */
5
struct git_attr;
6
7
+/* opaque structure used internally for attribute collection */
8
+struct all_attrs_item;
9
+
10
/*
11
* Given a string, return the gitattribute object that
12
* corresponds to it.
@@ -33,6 +36,8 @@ struct attr_check {
36
int nr;
37
int alloc;
38
struct attr_check_item *items;
39
+ int all_attrs_nr;
40
+ struct all_attrs_item *all_attrs;
41
};
42
43
extern struct attr_check *attr_check_alloc(void);