| Message ID | YBJW0gRn9SnP6N9d@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | convert hash_pos() to oid_pos() | expand |
On Thu, Jan 28, 2021 at 01:16:50AM -0500, Jeff King wrote: > Note that "get_sha1_hex()" is a confusing historical name. It is > actually using the_hash_algo, so it would be sha256 in a sha256 repo. > We'll switch to using parse_oid_hex(), because that conveniently > advances our pointer. But it also gets rid of the sha1 name. Arguably > it's a little funny to use "object_id" here for something that isn't > actually naming an object, but it's unlikely to be a problem (and is > contained in a single function). Not that it matters, but I think the argument would be that having "oid" in the function call probably isn't ideal, but it's closer to the ideal than having "sha1" in the same call (which isn't always the case). Likewise about advancing the pointer, since everything is pass-by-value, modifying the stack local copy of path doesn't matter to callers of is_rr_cache_dirname(). So I think that this change is an improvement over the status-quo here. Thanks, Taylor
diff --git a/rerere.c b/rerere.c index 7b0c262ac6..e92e305f96 100644 --- a/rerere.c +++ b/rerere.c @@ -1181,8 +1181,9 @@ static void prune_one(struct rerere_id *id, /* Does the basename in "path" look plausibly like an rr-cache entry? */ static int is_rr_cache_dirname(const char *path) { - unsigned char hash[GIT_MAX_RAWSZ]; - return !get_sha1_hex(path, hash); + struct object_id oid; + const char *end; + return !parse_oid_hex(path, &oid, &end) && !*end; } void rerere_gc(struct repository *r, struct string_list *rr)
We check only that get_sha1_hex() doesn't complain, which means we'd match an all-hex name with trailing cruft after it. This probably doesn't matter much in practice, since there shouldn't be anything else ni the rr-cache directory, but it could possibly cause us to mix up sha1 and sha256 entries (which also shouldn't be intermingled, but could be leftovers from a repository conversion). Note that "get_sha1_hex()" is a confusing historical name. It is actually using the_hash_algo, so it would be sha256 in a sha256 repo. We'll switch to using parse_oid_hex(), because that conveniently advances our pointer. But it also gets rid of the sha1 name. Arguably it's a little funny to use "object_id" here for something that isn't actually naming an object, but it's unlikely to be a problem (and is contained in a single function). Signed-off-by: Jeff King <peff@peff.net> --- This is mostly just an oddity I noticed while touching the code, and I've never seen triggered in real life. rerere.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)