Re: [PATCH 06/13] fsck: stop using object_info->type_name strbuf

From: Jeff King <peff@peff.net>
Date: May 19, 2025, 17:00 UTC
Message-ID: <20250519170032.GA772062@coredump.intra.peff.net>
In-Reply-To: <xmqqy0uspthe.fsf@gitster.g>

On Mon, May 19, 2025 at 07:26:05AM -0700, Junio C Hamano wrote:

Show 18 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > So we can simplify things by bailing immediately in read_loose_object()
> > when we encounter an unknown type. This has a few user-visible effects:
> >
> >   a. Instead of producing a single line of error output like this:
> >
> >        error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object is of unknown type 'bogus': .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6
> >
> >      we'll now issue two lines (the first from read_loose_object() when
> >      we see the unparsable header, and the second from the fsck code,
> >      since we couldn't read the object):
> >
> >        error: unable to parse type from header 'bogus 4' of .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6
> >        error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object corrupt or missing: .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6
> 
> Just curious; is the difference between 'bogus' and 'bogus 4'
> significant in the above examples?

The "4" is the size of the object. Knowing me, it was almost certainly "foo\n". ;)

In the original, we had parsed the type name into its own strbuf, so the error message reported that. Now we just get OBJ_BAD, but we still have the full header sitting in a buffer (we unpack_loose_header() to zlib inflate up until the NUL byte, and then we parse_loose_header() to actually interpret it).

We _could_ do something like:

  p = strchrnul(hdr, ' ');
  *p = '\0'; /* truncate header at end of type */
  error("bad type: %s", hdr);

to just print "bogus". But showing the whole header is less work, and arguably could be more informative, depending on exactly how it's malformed.

Note that we _are_ guaranteed to have a NUL byte, or else unpack_loose_header() would have complained and we'd have said:

  error: unable to unpack header of .git/objects/...

That's what you see if you put in a very long type name (we only inflate up to MAX_HEADER_LEN, so we never find the NUL).

-Peff