Re: [PATCH v2 5/6] fsck: report invalid types recorded in objects

From: Jeff King <peff@peff.net>
Date: Apr 23, 2021, 14:37 UTC
Message-ID: <YILbk34nwqCPmxGQ@coredump.intra.peff.net>
In-Reply-To: <patch-5.6-5fb6ac4faee-20210413T093734Z-avarab@gmail.com>

On Tue, Apr 13, 2021 at 11:43:08AM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 13 quoted lines

> Continue the work in the preceding commit and improve the error on:
> 
>     $ git hash-object --stdin -w -t garbage --literally </dev/null
>     $ git fsck
>     error: hash mismatch for <OID_PATH> (expected <OID>)
>     error: <OID>: object corrupt or missing: <OID_PATH>
>     [ other fsck output ]
> 
> To instead emit:
> 
>     $ git fsck
>     error: <OID>: object is of unknown type 'garbage': <OID_PATH>
>     [ other fsck output ]

Sounds good.

Show 10 quoted lines

> @@ -92,7 +93,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
>  	switch (opt) {
>  	case 't':
>  		oi.type_name = &sb;
> -		if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
> +		ret = oid_object_info_extended(the_repository, &oid, &oi, flags);
> +		if (!unknown_type && ret < 0)
>  			die("git cat-file: could not get object info");
>  		if (sb.len) {
>  			printf("%s\n", sb.buf);

Surprised to see changes to cat-file here, since the commit message is all about fsck. Did the semantics of oid_object_info_extended() change? I.e., this hunk implies to me that it is now returning -1 when we said unknown types were OK, and we got one. But in that case, how do we distinguish that from a real error?

Or more concretely, this patch causes this:

  $ git cat-file -t 1234567890123456789012345678901234567890
  fatal: git cat-file: could not get object info

  $ git.compile cat-file --allow-unknown-type -t 1234567890123456789012345678901234567890
  fatal: git cat-file 1234567890123456789012345678901234567890: bad file

Or much worse, from the next hunk:

  $ git cat-file -s 1234567890123456789012345678901234567890
  fatal: git cat-file: could not get object info

  $ git cat-file --allow-unknown-type -s 1234567890123456789012345678901234567890
  140732113568960

That seems wrong (so I think my "this hunk implies" is not true, but then I am left with: what is the point of this hunk?).

-Peff