Skip to content

Commit a5e92ab

Browse files
author
Junio C Hamano
committed
Fix funny types used in attribute value representation
It was bothering me a lot that I abused small integer values casted to (void *) to represent non string values in gitattributes. This corrects it by making the type of attribute values (const char *), and using the address of a few statically allocated character buffer to denote true/false. Unset attributes are represented as having NULLs as their values. Added in-header documentation to explain how git_checkattr() routine should be called. Signed-off-by: Junio C Hamano <junkio@cox.net>
1 parent 3086486 commit a5e92ab

File tree

6 files changed

+43
-26
lines changed

6 files changed

+43
-26
lines changed

attr.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
#include "cache.h"
22
#include "attr.h"
33

4-
#define ATTR__UNKNOWN ((void *) -2)
4+
const char git_attr__true[] = "(builtin)true";
5+
const char git_attr__false[] = "\0(builtin)false";
6+
static const char git_attr__unknown[] = "(builtin)unknown";
7+
#define ATTR__TRUE git_attr__true
8+
#define ATTR__FALSE git_attr__false
9+
#define ATTR__UNSET NULL
10+
#define ATTR__UNKNOWN git_attr__unknown
511

612
/*
713
* The basic design decision here is that we are not going to have
@@ -102,7 +108,7 @@ struct git_attr *git_attr(const char *name, int len)
102108
/* What does a matched pattern decide? */
103109
struct attr_state {
104110
struct git_attr *attr;
105-
void *setto;
111+
const char *setto;
106112
};
107113

108114
struct match_attr {
@@ -262,14 +268,14 @@ static void free_attr_elem(struct attr_stack *e)
262268
struct match_attr *a = e->attrs[i];
263269
int j;
264270
for (j = 0; j < a->num_attr; j++) {
265-
void *setto = a->state[j].setto;
271+
const char *setto = a->state[j].setto;
266272
if (setto == ATTR__TRUE ||
267273
setto == ATTR__FALSE ||
268274
setto == ATTR__UNSET ||
269275
setto == ATTR__UNKNOWN)
270276
;
271277
else
272-
free(setto);
278+
free((char*) setto);
273279
}
274280
free(a);
275281
}
@@ -478,8 +484,8 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
478484

479485
for (i = 0; 0 < rem && i < a->num_attr; i++) {
480486
struct git_attr *attr = a->state[i].attr;
481-
void **n = &(check[attr->attr_nr].value);
482-
void *v = a->state[i].setto;
487+
const char **n = &(check[attr->attr_nr].value);
488+
const char *v = a->state[i].setto;
483489

484490
if (*n == ATTR__UNKNOWN) {
485491
debug_set(what, a->u.pattern, attr, v);
@@ -547,7 +553,7 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)
547553
rem = macroexpand(stk, rem);
548554

549555
for (i = 0; i < num; i++) {
550-
void *value = check_all_attr[check[i].attr->attr_nr].value;
556+
const char *value = check_all_attr[check[i].attr->attr_nr].value;
551557
if (value == ATTR__UNKNOWN)
552558
value = ATTR__UNSET;
553559
check[i].value = value;

attr.h

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

7+
/*
8+
* Given a string, return the gitattribute object that
9+
* corresponds to it.
10+
*/
711
struct git_attr *git_attr(const char *, int);
812

913
/* Internal use */
10-
#define ATTR__TRUE ((void *) 1)
11-
#define ATTR__FALSE ((void *) 0)
12-
#define ATTR__UNSET ((void *) -1)
14+
extern const char git_attr__true[];
15+
extern const char git_attr__false[];
1316

1417
/* For public to check git_attr_check results */
15-
#define ATTR_TRUE(v) ((v) == ATTR__TRUE)
16-
#define ATTR_FALSE(v) ((v) == ATTR__FALSE)
17-
#define ATTR_UNSET(v) ((v) == ATTR__UNSET)
18+
#define ATTR_TRUE(v) ((v) == git_attr__true)
19+
#define ATTR_FALSE(v) ((v) == git_attr__false)
20+
#define ATTR_UNSET(v) ((v) == NULL)
1821

22+
/*
23+
* Send one or more git_attr_check to git_checkattr(), and
24+
* each 'value' member tells what its value is.
25+
* Unset one is returned as NULL.
26+
*/
1927
struct git_attr_check {
2028
struct git_attr *attr;
21-
void *value;
29+
const char *value;
2230
};
2331

2432
int git_checkattr(const char *path, int, struct git_attr_check *);

builtin-check-attr.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
4242
if (git_checkattr(argv[i], cnt, check))
4343
die("git_checkattr died");
4444
for (j = 0; j < cnt; j++) {
45-
void *value = check[j].value;
45+
const char *value = check[j].value;
4646

4747
if (ATTR_TRUE(value))
4848
value = "set";
@@ -52,7 +52,7 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
5252
value = "unspecified";
5353

5454
write_name_quoted("", 0, argv[i], 1, stdout);
55-
printf(": %s: %s\n", argv[j+1], (char *) value);
55+
printf(": %s: %s\n", argv[j+1], value);
5656
}
5757
}
5858
return 0;

convert.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ static int git_path_check_crlf(const char *path)
226226
setup_crlf_check(&attr_crlf_check);
227227

228228
if (!git_checkattr(path, 1, &attr_crlf_check)) {
229-
void *value = attr_crlf_check.value;
229+
const char *value = attr_crlf_check.value;
230230
if (ATTR_TRUE(value))
231231
return 1;
232232
else if (ATTR_FALSE(value))

diff.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,7 +1069,7 @@ static int file_is_binary(struct diff_filespec *one)
10691069

10701070
setup_diff_attr_check(&attr_diff_check);
10711071
if (!git_checkattr(one->path, 1, &attr_diff_check)) {
1072-
void *value = attr_diff_check.value;
1072+
const char *value = attr_diff_check.value;
10731073
if (ATTR_TRUE(value))
10741074
return 0;
10751075
else if (ATTR_FALSE(value))
@@ -1078,7 +1078,7 @@ static int file_is_binary(struct diff_filespec *one)
10781078
;
10791079
else
10801080
die("unknown value %s given to 'diff' attribute",
1081-
(char *)value);
1081+
value);
10821082
}
10831083

10841084
if (!one->data) {

merge-recursive.c

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -953,7 +953,7 @@ static void initialize_ll_merge(void)
953953
git_config(read_merge_config);
954954
}
955955

956-
static const struct ll_merge_driver *find_ll_merge_driver(void *merge_attr)
956+
static const struct ll_merge_driver *find_ll_merge_driver(const char *merge_attr)
957957
{
958958
struct ll_merge_driver *fn;
959959
const char *name;
@@ -986,15 +986,15 @@ static const struct ll_merge_driver *find_ll_merge_driver(void *merge_attr)
986986
return &ll_merge_drv[LL_TEXT_MERGE];
987987
}
988988

989-
static void *git_path_check_merge(const char *path)
989+
static const char *git_path_check_merge(const char *path)
990990
{
991991
static struct git_attr_check attr_merge_check;
992992

993993
if (!attr_merge_check.attr)
994994
attr_merge_check.attr = git_attr("merge", 5);
995995

996996
if (git_checkattr(path, 1, &attr_merge_check))
997-
return ATTR__UNSET;
997+
return NULL;
998998
return attr_merge_check.value;
999999
}
10001000

@@ -1008,7 +1008,7 @@ static int ll_merge(mmbuffer_t *result_buf,
10081008
mmfile_t orig, src1, src2;
10091009
char *name1, *name2;
10101010
int merge_status;
1011-
void *merge_attr;
1011+
const char *ll_driver_name;
10121012
const struct ll_merge_driver *driver;
10131013

10141014
name1 = xstrdup(mkpath("%s:%s", branch1, a->path));
@@ -1018,11 +1018,14 @@ static int ll_merge(mmbuffer_t *result_buf,
10181018
fill_mm(a->sha1, &src1);
10191019
fill_mm(b->sha1, &src2);
10201020

1021-
merge_attr = git_path_check_merge(a->path);
1022-
driver = find_ll_merge_driver(merge_attr);
1021+
ll_driver_name = git_path_check_merge(a->path);
1022+
driver = find_ll_merge_driver(ll_driver_name);
10231023

10241024
if (index_only && driver->recursive) {
1025-
merge_attr = git_attr(driver->recursive, strlen(driver->recursive));
1025+
void *merge_attr;
1026+
1027+
ll_driver_name = driver->recursive;
1028+
merge_attr = git_attr(ll_driver_name, strlen(ll_driver_name));
10261029
driver = find_ll_merge_driver(merge_attr);
10271030
}
10281031
merge_status = driver->fn(driver, a->path,

0 commit comments

Comments
 (0)