Re: [PATCH v2 2/7] gc: convert to using the_hash_algo

On Fri, Mar 15 2019, Duy Nguyen wrote:

On Thu, Mar 14, 2019 at 7:34 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

quoted

There's been a lot of changing of the hardcoded "40" values to
the_hash_algo->hexsz, but we've so far missed this one where we
hardcoded 38 for the loose object file length.

Wow. Good catch.

quoted

diff --git a/builtin/gc.c b/builtin/gc.c
index 8c2312681c..733bd7bdf4 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c

@@ -156,6 +156,7 @@ static int too_many_loose_objects(void)
        int auto_threshold;
        int num_loose = 0;
        int needed = 0;
+       const unsigned hexsz_loose = the_hash_algo->hexsz - 2;

Since you're removing hard coded numbers. Another option is a
combination of strlen(basename()) and
loose_object_path(the_repository, , null_oid). They should also give
the same 38. Then if loose object format is updated to use 3+ chars
for the directory part, this code will not need update anymore.

The downside of course is a lot more code. Or you can just introduce a
new function to return this "hexsz - 2", keep that function close to
fill_loose_path() with a comment there that the two functions should
be aligned.

I think it's better to just keep hardcoding "2". We're very unlikely to
ever change objects/?? in favor of e.g. objects/???, and if we were that
would be a huge "hash function transition" of its own.

quoted

        dir = opendir(git_path("objects/17"));
        if (!dir)