Re: Public repro case! Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file

On Wed, 7 Jan 2009, R. Tyler Ballance wrote:

        tyler@grapefruit:~/source/git/linux-2.6> git pull
        error: failed to read object be1b87c70af69acfadb8a27a7a76dfb61de92643 at offset 1850923
        from .git/objects/pack/pack-dbe154052997a05499eb6b4fd90b924da68e799a.pack
        fatal: object be1b87c70af69acfadb8a27a7a76dfb61de92643 is corrupted

Btw, this is an interesting error message, mostly because of what is 
_not_ there.

In particular, it doesn't report any reason _why_ it failed to read the 
object, which as far as I can tell can happen for only one reason: 
unpack_compressed_entry() returns NULL, and that path is the only thing 
that can do so without a message.

And it only does it if zlib fails.

Now, zlib can fail because the unpacking fails, but it can fail for other 
cases too.

And the thing is, we don't check/report those kinds of failure cases very 
well. Which really bit us here, because if we had checked the return value 
of inflateInit(), we'd almost certainly would have gotten a nice big "you 
ran out of memory" thing, and we wouldn't have been chasing this down as a 
corruption issue.

We probably should wrap all the "inflateInit()" calls, and do something 
like

	static void xinflateInit(z_streamp strm)
	{
		const char *err;

		switch (inflateInit(strm)) {
		case Z_OK:
			return;
		case Z_MEM_ERROR:
			err = "out of memory";
			break;
		case Z_VERSION_ERROR:
			err = "wrong version";
			break;
		default:
			err = "error";
		}
		die("inflateInit: %s (%s)", err,
			strm->msg ? strm->msg : "no message");
	}

or similar. That way we'd get good error reports when we run out of 
memory, rather than consider it to be a corruption issue.

We could also try to make a few of these wrappers actually release some of 
the memory (the way xmmap() does), but there are likely diminishing 
returns. And the much more important issue is the proper error reporting: 
if we had reported "out of memory", we'd not have spent so much time 
chasing disk corruption etc.

			Linus