public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Derrick Stolee <derrickstolee@github•com>
To: Junio C Hamano <gitster@pobox•com>,
	Derrick Stolee via GitGitGadget <gitgitgadget@gmail•com>
Cc: git@vger•kernel.org, me@ttaylorr•com
Subject: Re: [PATCH 3/4] fsck: check rev-index position values
Date: Tue, 18 Apr 2023 10:32:13 -0400	[thread overview]
Message-ID: <e29e6584-709f-d172-e73a-88a2027a9283@github.com> (raw)
In-Reply-To: <xmqq1qkijhho.fsf@gitster.g>

On 4/17/2023 6:01 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail•com> writes:
> 
>>  	if (!hashfile_checksum_valid((const unsigned char *)p->revindex_map, p->revindex_size)) {
>>  		error(_("invalid checksum"));
>> -		return -1;
>> +		res = -1;
>>  	}
>>  
>> -	return 0;
>> +	/* This may fail due to a broken .idx. */
>> +	if (create_pack_revindex_in_memory(p))
>> +		return res;
> 
> Here, if the revindex file itself is self consistent, res would
> still be 0, so we will end up silently returning.  Is the idea that
> while whatever causes in-memory revindex fail to be computed is a
> concern from the point of view of the repository's health, it would
> be caught elsewhere as a problem for the <pack,idx> pair we are
> seeing here?

This is something I noticed during a test suite run, where the .idx
was corrupt, so the .rev file could not be checked. In this situation,
we are not using the .rev file for anything since Git processes would
fail to lookup objects in the associated packfile.

In this case, we have no way to validate the rest of the .rev file's
contents, but at least we checked its header and checksum, which is
the best we can hope to do in this case.

Thanks,
-Stolee

next prev parent reply	other threads:[~2023-04-18 14:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-17 16:21 [PATCH 0/4] git fsck: check pack rev-index files Derrick Stolee via GitGitGadget
2023-04-17 16:21 ` [PATCH 1/4] fsck: create scaffolding for rev-index checks Derrick Stolee via GitGitGadget
2023-04-17 22:20   ` Taylor Blau
2023-04-17 16:21 ` [PATCH 2/4] fsck: check rev-index checksums Derrick Stolee via GitGitGadget
2023-04-17 22:15   ` Junio C Hamano
2023-04-18 14:24     ` Derrick Stolee
2023-04-17 22:24   ` Taylor Blau
2023-04-18 14:27     ` Derrick Stolee
2023-04-18 14:51       ` Taylor Blau
2023-04-18 14:57         ` Derrick Stolee
2023-04-18 15:03           ` Taylor Blau
2023-04-17 16:21 ` [PATCH 3/4] fsck: check rev-index position values Derrick Stolee via GitGitGadget
2023-04-17 22:01   ` Junio C Hamano
2023-04-18 14:32     ` Derrick Stolee [this message]
2023-04-17 22:52   ` Taylor Blau
2023-04-17 16:21 ` [PATCH 4/4] fsck: validate .rev file header Derrick Stolee via GitGitGadget
2023-04-17 21:37 ` [PATCH 0/4] git fsck: check pack rev-index files Junio C Hamano
2023-04-18 15:23 ` Taylor Blau
2023-04-18 16:59   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e29e6584-709f-d172-e73a-88a2027a9283@github.com \
    --to=derrickstolee@github$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitgitgadget@gmail$(echo .)com \
    --cc=gitster@pobox$(echo .)com \
    --cc=me@ttaylorr$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox