Re: [PATCH] drop support for "experimental" loose objects
- From
Jeff King <peff@peff.net>
- Date
- Nov 24, 2013, 09:07 UTC
- Message-ID
- <20131124090743.GA495@sigill.intra.peff.net>
- In-Reply-To
- <20131124084444.GA23238@sigill.intra.peff.net>
On Sun, Nov 24, 2013 at 03:44:44AM -0500, Jeff King wrote:
Show 5 quoted lines
> 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:
[before] real 0m12.941s user 0m12.700s sys 0m0.244s
[after] 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.
-Peff
---
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 *sha1, 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 *sha1, 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)