Re: [PATCH 2/4] read-cache: add index.skipHash config option
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Dec 7, 2022, 22:25 UTC
- Message-ID
- <221207.864ju6alyg.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <5fb4b5a36ac806f3ee07a614bcb93df2c430507c.1670433958.git.gitgitgadget@gmail.com>
On Wed, Dec 07 2022, Derrick Stolee via GitGitGadget wrote:
Show 24 quoted lines
> From: Derrick Stolee <derrickstolee@github.com>
> [...]
> diff --git a/read-cache.c b/read-cache.c
> index 46f5e497b14..fb4d6fb6387 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1817,6 +1817,8 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)
> git_hash_ctx c;
> unsigned char hash[GIT_MAX_RAWSZ];
> int hdr_version;
> + unsigned char *start, *end;
> + struct object_id oid;
>
> if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
> return error(_("bad signature 0x%08x"), hdr->hdr_signature);
> @@ -1827,10 +1829,16 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)
> if (!verify_index_checksum)
> return 0;
>
> + end = (unsigned char *)hdr + size;
> + start = end - the_hash_algo->rawsz;
> + oidread(&oid, start);
> + if (oideq(&oid, null_oid()))
> + return 0;It's good to see this use the existing hashing support, as I suggested in the RFC comments. Glad it helped.
> int ieot_entries = 1; > struct index_entry_offset_table *ieot = NULL; > int nr, nr_threads; > + int skip_hash;
You don't need this variable.
Show 5 quoted lines
>
> f = hashfd(tempfile->fd, tempfile->filename.buf);
>
> + if (!git_config_get_maybe_bool("index.skiphash", &skip_hash))
> + f->skip_hash = skip_hash;Because this can just be:
git_config_get_maybe_bool("index.skiphash", &f->skip_hash);I.e. the config API guarantees that it won't touch the variable if the key doesn't exist, so no need for the intermediate variable.
In a later commit you convert this to that very API use when moving this to the repo-settings.
Can we maybe skip straight to that step?
Show 6 quoted lines
> +test_expect_success 'index.skipHash config option' ' > + ( > + rm -f .git/index && > + git -c index.skipHash=true add a && > + git fsck > + )
Why the subshell?