Introduce diff_filespec_is_binary()

This replaces an explicit initialization of filespec->is_binary
field used for rename/break followed by direct access to that
field with a wrapper function that lazily iniaitlizes and
accesses the field.  We would add more attribute accesses for
the use of diff routines, and it would be better to make this
abstraction earlier.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 19589707c4cf40e622b0f7b98b8fd07704b37c01..16ea7100dfba40f30b6f9024c90131cdcfaf9b4b 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1102,30 +1102,45 @@ static void setup_diff_attr_check(struct git_attr_check *check)
 {
        static struct git_attr *attr_diff;
 
 {
        static struct git_attr *attr_diff;
 

-       if (!attr_diff)
+       if (!attr_diff) {

                attr_diff = git_attr("diff", 4);
                attr_diff = git_attr("diff", 4);

-       check->attr = attr_diff;
+       }
+       check[0].attr = attr_diff;

 }
 
 }
 

-static int file_is_binary(struct diff_filespec *one)
+static void diff_filespec_check_attr(struct diff_filespec *one)

 {
 {

-       struct git_attr_check attr_diff_check;
+       struct git_attr_check attr_diff_check[1];

 
 

-       setup_diff_attr_check(&attr_diff_check);
-       if (!git_checkattr(one->path, 1, &attr_diff_check)) {
-               const char *value = attr_diff_check.value;
+       if (one->checked_attr)
+               return;
+
+       setup_diff_attr_check(attr_diff_check);
+       one->is_binary = 0;
+
+       if (!git_checkattr(one->path, ARRAY_SIZE(attr_diff_check), attr_diff_check)) {
+               const char *value;
+
+               /* binaryness */
+               value = attr_diff_check[0].value;

                if (ATTR_TRUE(value))
                if (ATTR_TRUE(value))

-                       return 0;
+                       ;

                else if (ATTR_FALSE(value))
                else if (ATTR_FALSE(value))

-                       return 1;
+                       one->is_binary = 1;

        }
 
        }
 

-       if (!one->data) {
-               if (!DIFF_FILE_VALID(one))
-                       return 0;
+       if (!one->data && DIFF_FILE_VALID(one))

                diff_populate_filespec(one, 0);
                diff_populate_filespec(one, 0);

-       }
-       return buffer_is_binary(one->data, one->size);
+
+       if (one->data)
+               one->is_binary = buffer_is_binary(one->data, one->size);
+
+}
+
+int diff_filespec_is_binary(struct diff_filespec *one)
+{
+       diff_filespec_check_attr(one);
+       return one->is_binary;

 }
 
 static void builtin_diff(const char *name_a,
 }
 
 static void builtin_diff(const char *name_a,


@@ -1182,7 +1197,8 @@ static void builtin_diff(const char *name_a,
        if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
                die("unable to read files to diff");
 
        if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
                die("unable to read files to diff");
 

-       if (!o->text && (file_is_binary(one) || file_is_binary(two))) {
+       if (!o->text &&
+           (diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) {

                /* Quite common confusing case */
                if (mf1.size == mf2.size &&
                    !memcmp(mf1.ptr, mf2.ptr, mf1.size))
                /* Quite common confusing case */
                if (mf1.size == mf2.size &&
                    !memcmp(mf1.ptr, mf2.ptr, mf1.size))


@@ -1260,7 +1276,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
        if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
                die("unable to read files to diff");
 
        if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
                die("unable to read files to diff");
 

-       if (file_is_binary(one) || file_is_binary(two)) {
+       if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {

                data->is_binary = 1;
                data->added = mf2.size;
                data->deleted = mf1.size;
                data->is_binary = 1;
                data->added = mf2.size;
                data->deleted = mf1.size;


@@ -1302,7 +1318,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
        if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
                die("unable to read files to diff");
 
        if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
                die("unable to read files to diff");
 

-       if (file_is_binary(two))
+       if (diff_filespec_is_binary(two))

                goto free_and_return;
        else {
                /* Crazy xdl interfaces.. */
                goto free_and_return;
        else {
                /* Crazy xdl interfaces.. */


@@ -1880,8 +1896,8 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 
                if (o->binary) {
                        mmfile_t mf;
 
                if (o->binary) {
                        mmfile_t mf;

-                       if ((!fill_mmfile(&mf, one) && file_is_binary(one)) ||
-                           (!fill_mmfile(&mf, two) && file_is_binary(two)))
+                       if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
+                           (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))

                                abbrev = 40;
                }
                len += snprintf(msg + len, sizeof(msg) - len,
                                abbrev = 40;
                }
                len += snprintf(msg + len, sizeof(msg) - len,


@@ -2783,7 +2799,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
                        return error("unable to read files to diff");
 
                /* Maybe hash p->two? into the patch id? */
                        return error("unable to read files to diff");
 
                /* Maybe hash p->two? into the patch id? */

-               if (file_is_binary(p->two))
+               if (diff_filespec_is_binary(p->two))

                        continue;
 
                len1 = remove_space(p->one->path, strlen(p->one->path));
                        continue;
 
                len1 = remove_space(p->one->path, strlen(p->one->path));


@@ -3011,21 +3027,6 @@ void diffcore_std(struct diff_options *options)
        if (options->quiet)
                return;
 
        if (options->quiet)
                return;
 

-       /*
-        * break/rename count similarity differently depending on
-        * the binary-ness.
-        */
-       if ((options->break_opt != -1) || (options->detect_rename)) {
-               struct diff_queue_struct *q = &diff_queued_diff;
-               int i;
-
-               for (i = 0; i < q->nr; i++) {
-                       struct diff_filepair *p = q->queue[i];
-                       p->one->is_binary = file_is_binary(p->one);
-                       p->two->is_binary = file_is_binary(p->two);
-               }
-       }
-

        if (options->break_opt != -1)
                diffcore_break(options->break_opt);
        if (options->detect_rename)
        if (options->break_opt != -1)
                diffcore_break(options->break_opt);
        if (options->detect_rename)

diff --git a/diffcore-delta.c b/diffcore-delta.c
index a038b166c5aa095ffdf31c2fb0e3263ed13cf8b1..d9729e5ec27d2ff2bacf4ed930195f9293f8ca5a 100644 (file)
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -129,7 +129,7 @@ static struct spanhash_top *hash_chars(struct diff_filespec *one)
        struct spanhash_top *hash;
        unsigned char *buf = one->data;
        unsigned int sz = one->size;
        struct spanhash_top *hash;
        unsigned char *buf = one->data;
        unsigned int sz = one->size;

-       int is_text = !one->is_binary;
+       int is_text = !diff_filespec_is_binary(one);

 
        i = INITIAL_HASH_SIZE;
        hash = xmalloc(sizeof(*hash) + sizeof(struct spanhash) * (1<<i));
 
        i = INITIAL_HASH_SIZE;
        hash = xmalloc(sizeof(*hash) + sizeof(struct spanhash) * (1<<i));

diff --git a/diffcore.h b/diffcore.h
index 0c8abb5b94dc07769e5cd2603988dad6e6869bb7..dcab7e20bf6922a17f93ab268376d57765a8c6b8 100644 (file)
--- a/diffcore.h
+++ b/diffcore.h
@@ -37,6 +37,7 @@ struct diff_filespec {
 #define DIFF_FILE_VALID(spec) (((spec)->mode) != 0)
        unsigned should_free : 1; /* data should be free()'ed */
        unsigned should_munmap : 1; /* data should be munmap()'ed */
 #define DIFF_FILE_VALID(spec) (((spec)->mode) != 0)
        unsigned should_free : 1; /* data should be free()'ed */
        unsigned should_munmap : 1; /* data should be munmap()'ed */

+       unsigned checked_attr : 1;

        unsigned is_binary : 1; /* data should be considered "binary" */
 };
 
        unsigned is_binary : 1; /* data should be considered "binary" */
 };
 


@@ -46,6 +47,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *,
 
 extern int diff_populate_filespec(struct diff_filespec *, int);
 extern void diff_free_filespec_data(struct diff_filespec *);
 
 extern int diff_populate_filespec(struct diff_filespec *, int);
 extern void diff_free_filespec_data(struct diff_filespec *);

+extern int diff_filespec_is_binary(struct diff_filespec *);

 
 struct diff_filepair {
        struct diff_filespec *one;
 
 struct diff_filepair {
        struct diff_filespec *one;