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>

diff --git a/attr.c b/attr.c
index b3496a6eb529c111ff0378a517b13ca89cb74d5a..285e689e5adff6237e179f5d64f11d3f3c09a09d 100644 (file)
--- a/attr.c
+++ b/attr.c
@@ -1,7 +1,13 @@
 #include "cache.h"
 #include "attr.h"
 
-#define ATTR__UNKNOWN  ((void *) -2)
+const char git_attr__true[] = "(builtin)true";
+const char git_attr__false[] = "\0(builtin)false";
+static const char git_attr__unknown[] = "(builtin)unknown";
+#define ATTR__TRUE git_attr__true
+#define ATTR__FALSE git_attr__false
+#define ATTR__UNSET NULL
+#define ATTR__UNKNOWN git_attr__unknown
 
 /*
  * 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)
 /* What does a matched pattern decide? */
 struct attr_state {
        struct git_attr *attr;
-       void *setto;
+       const char *setto;
 };
 
 struct match_attr {
@@ -262,14 +268,14 @@ static void free_attr_elem(struct attr_stack *e)
                struct match_attr *a = e->attrs[i];
                int j;
                for (j = 0; j < a->num_attr; j++) {
-                       void *setto = a->state[j].setto;
+                       const char *setto = a->state[j].setto;
                        if (setto == ATTR__TRUE ||
                            setto == ATTR__FALSE ||
                            setto == ATTR__UNSET ||
                            setto == ATTR__UNKNOWN)
                                ;
                        else
-                               free(setto);
+                               free((char*) setto);
                }
                free(a);
        }
@@ -478,8 +484,8 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
 
        for (i = 0; 0 < rem && i < a->num_attr; i++) {
                struct git_attr *attr = a->state[i].attr;
-               void **n = &(check[attr->attr_nr].value);
-               void *v = a->state[i].setto;
+               const char **n = &(check[attr->attr_nr].value);
+               const char *v = a->state[i].setto;
 
                if (*n == ATTR__UNKNOWN) {
                        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)
                rem = macroexpand(stk, rem);
 
        for (i = 0; i < num; i++) {
-               void *value = check_all_attr[check[i].attr->attr_nr].value;
+               const char *value = check_all_attr[check[i].attr->attr_nr].value;
                if (value == ATTR__UNKNOWN)
                        value = ATTR__UNSET;
                check[i].value = value;

diff --git a/attr.h b/attr.h
index 8ec2d3d35ce02d15adb0c3b4c72ce2d435d5a1ae..f1c2038b0923d3130937eef965667204a8634e6d 100644 (file)
--- a/attr.h
+++ b/attr.h
@@ -4,21 +4,29 @@
 /* An attribute is a pointer to this opaque structure */
 struct git_attr;
 
+/*
+ * Given a string, return the gitattribute object that
+ * corresponds to it.
+ */
 struct git_attr *git_attr(const char *, int);
 
 /* Internal use */
-#define ATTR__TRUE     ((void *) 1)
-#define ATTR__FALSE    ((void *) 0)
-#define ATTR__UNSET    ((void *) -1)
+extern const char git_attr__true[];
+extern const char git_attr__false[];
 
 /* For public to check git_attr_check results */
-#define ATTR_TRUE(v) ((v) == ATTR__TRUE)
-#define ATTR_FALSE(v) ((v) == ATTR__FALSE)
-#define ATTR_UNSET(v) ((v) == ATTR__UNSET)
+#define ATTR_TRUE(v) ((v) == git_attr__true)
+#define ATTR_FALSE(v) ((v) == git_attr__false)
+#define ATTR_UNSET(v) ((v) == NULL)
 
+/*
+ * Send one or more git_attr_check to git_checkattr(), and
+ * each 'value' member tells what its value is.
+ * Unset one is returned as NULL.
+ */
 struct git_attr_check {
        struct git_attr *attr;
-       void *value;
+       const char *value;
 };
 
 int git_checkattr(const char *path, int, struct git_attr_check *);

diff --git a/builtin-check-attr.c b/builtin-check-attr.c
index 6983a73c1b387e5eb4a0c8dbc6b9e72b93735245..9d77f76ff1230df76278b528ef44d6837edb2741 100644 (file)
--- a/builtin-check-attr.c
+++ b/builtin-check-attr.c
@@ -42,7 +42,7 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
                if (git_checkattr(argv[i], cnt, check))
                        die("git_checkattr died");
                for (j = 0; j < cnt; j++) {
-                       void *value = check[j].value;
+                       const char *value = check[j].value;
 
                        if (ATTR_TRUE(value))
                                value = "set";
@@ -52,7 +52,7 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
                                value = "unspecified";
 
                        write_name_quoted("", 0, argv[i], 1, stdout);
-                       printf(": %s: %s\n", argv[j+1], (char *) value);
+                       printf(": %s: %s\n", argv[j+1], value);
                }
        }
        return 0;

diff --git a/convert.c b/convert.c
index 68bb70f12ff49fede13bce01e2d8a2f0d64734d1..a5f60c7c6b729efbc7b98b1f5ac098cbf1bf9728 100644 (file)
--- a/convert.c
+++ b/convert.c
@@ -226,7 +226,7 @@ static int git_path_check_crlf(const char *path)
        setup_crlf_check(&attr_crlf_check);
 
        if (!git_checkattr(path, 1, &attr_crlf_check)) {
-               void *value = attr_crlf_check.value;
+               const char *value = attr_crlf_check.value;
                if (ATTR_TRUE(value))
                        return 1;
                else if (ATTR_FALSE(value))

diff --git a/diff.c b/diff.c
index a32078ea0177cb2a57e4a470bf6e50c81d6dd70d..5f501864e60899c7cdcaedeadaf996310a74be7b 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1069,7 +1069,7 @@ static int file_is_binary(struct diff_filespec *one)
 
        setup_diff_attr_check(&attr_diff_check);
        if (!git_checkattr(one->path, 1, &attr_diff_check)) {
-               void *value = attr_diff_check.value;
+               const char *value = attr_diff_check.value;
                if (ATTR_TRUE(value))
                        return 0;
                else if (ATTR_FALSE(value))
@@ -1078,7 +1078,7 @@ static int file_is_binary(struct diff_filespec *one)
                        ;
                else
                        die("unknown value %s given to 'diff' attribute",
-                           (char *)value);
+                           value);
        }
 
        if (!one->data) {

diff --git a/merge-recursive.c b/merge-recursive.c
index 7b5ca8e717217011ffde95434e58d47e401c40e5..ec8438b463a5b096ceeca916126e440198ff4740 100644 (file)
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -953,7 +953,7 @@ static void initialize_ll_merge(void)
        git_config(read_merge_config);
 }
 
-static const struct ll_merge_driver *find_ll_merge_driver(void *merge_attr)
+static const struct ll_merge_driver *find_ll_merge_driver(const char *merge_attr)
 {
        struct ll_merge_driver *fn;
        const char *name;
@@ -986,7 +986,7 @@ static const struct ll_merge_driver *find_ll_merge_driver(void *merge_attr)
        return &ll_merge_drv[LL_TEXT_MERGE];
 }
 
-static void *git_path_check_merge(const char *path)
+static const char *git_path_check_merge(const char *path)
 {
        static struct git_attr_check attr_merge_check;
 
@@ -994,7 +994,7 @@ static void *git_path_check_merge(const char *path)
                attr_merge_check.attr = git_attr("merge", 5);
 
        if (git_checkattr(path, 1, &attr_merge_check))
-               return ATTR__UNSET;
+               return NULL;
        return attr_merge_check.value;
 }
 
@@ -1008,7 +1008,7 @@ static int ll_merge(mmbuffer_t *result_buf,
        mmfile_t orig, src1, src2;
        char *name1, *name2;
        int merge_status;
-       void *merge_attr;
+       const char *ll_driver_name;
        const struct ll_merge_driver *driver;
 
        name1 = xstrdup(mkpath("%s:%s", branch1, a->path));
@@ -1018,11 +1018,14 @@ static int ll_merge(mmbuffer_t *result_buf,
        fill_mm(a->sha1, &src1);
        fill_mm(b->sha1, &src2);
 
-       merge_attr = git_path_check_merge(a->path);
-       driver = find_ll_merge_driver(merge_attr);
+       ll_driver_name = git_path_check_merge(a->path);
+       driver = find_ll_merge_driver(ll_driver_name);
 
        if (index_only && driver->recursive) {
-               merge_attr = git_attr(driver->recursive, strlen(driver->recursive));
+               void *merge_attr;
+
+               ll_driver_name = driver->recursive;
+               merge_attr = git_attr(ll_driver_name, strlen(ll_driver_name));
                driver = find_ll_merge_driver(merge_attr);
        }
        merge_status = driver->fn(driver, a->path,