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

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 15, 2019, 09:51 UTC
Message-ID: <CACsJy8CikZt_WF79V6y2JRtR0jAosTPPmDBXN8uMRTjm4FAV-A@mail.gmail.com>
In-Reply-To: <20190314123439.4347-3-avarab@gmail.com>

On Thu, Mar 14, 2019 at 7:34 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

>
> 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.

Show 9 quoted lines

> 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.

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

-- 
Duy