On Sun, Nov 24, 2013 at 03:44:44AM -0500, Jeff King wrote:

> In any code path where we call parse_object, we double-check that the
> result matches the sha1 we asked for. But low-level commands like
> cat-file just call read_sha1_file directly, and do not have such a
> check. We could add it, but I suspect the processing cost would be
> noticeable.

Curious, I tested this. It is noticeable. Here's the best-of-five
timings for the patch below when running a --batch cat-file on every
object in my git.git repo, using the patch below:

  real    0m12.941s
  user    0m12.700s
  sys     0m0.244s

  real    0m15.800s
  user    0m15.472s
  sys     0m0.344s

So it's about 20% slower. I don't know what the right tradeoff is. It's
cool to check the data each time we look at it, but it does carry a
performance penalty. I notice elsewhere in git we are inconsistent. If
you call parse_object() on an object, you get the sha1 check. But if you
just call parse_commit() on something you know to be a commit (e.g.,
because you are traversing the history and looked it up as a parent
pointer), you do not. I don't know if that is oversight, or an
intentional performance decision.


diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b2ca775..2b09773 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -199,6 +199,8 @@ static void print_object_or_die(int fd, const unsigned char 
        if (type == OBJ_BLOB) {
                if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0)
                        die("unable to stream %s to stdout", sha1_to_hex(sha1));
+               if (check_sha1_signature(sha1, NULL, 0, NULL) < 0)
+                       die("object %s sha1 mismatch", sha1_to_hex(sha1));
        else {
                enum object_type rtype;
@@ -208,6 +210,8 @@ static void print_object_or_die(int fd, const unsigned char 
                contents = read_sha1_file(sha1, &rtype, &rsize);
                if (!contents)
                        die("object %s disappeared", sha1_to_hex(sha1));
+               if (check_sha1_signature(sha1, contents, rsize, 
typename(rtype)) < 0)
+                       die("object %s sha1 mismatch", sha1_to_hex(sha1));
                if (rtype != type)
                        die("object %s changed type!?", sha1_to_hex(sha1));
                if (rsize != size)
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to