From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mon, 25 Mar 2013 21:10:38 GMT
Subject: Re: [PATCH 5/9] add test for streaming corrupt blobs
Message-ID: <20130325211038.GD1414@google.com>
In-Reply-To: <20130325202134.GE16019@sigill.intra.peff.net>

Jeff King wrote:

> We do not have many tests for handling corrupt objects. This
> new test at least checks that we detect a byte error in a
> corrupt blob object while streaming it out with cat-file.

Thanks.

[...]
> +# convert "1234abcd" to ".git/objects/12/34abcd"
> +obj_to_file() {
> +	echo "$(git rev-parse --git-dir)/objects/$(git rev-parse "$1" | sed 's,..,&/,')"
> +}

Maybe this would be clearer in multiple lines?

	commit=$(git rev-parse --verify "$1") &&
	git_dir=$(git rev-parse --git-dir) &&
	tail=${commit#??} &&
	echo "$git_dir/objects/${commit%$tail}/$tail"

> +
> +# Convert byte at offset "$2" of object "$1" into '\0'
> +corrupt_byte() {
> +	obj_file=$(obj_to_file "$1") &&
> +	chmod +w "$obj_file" &&
> +	printf '\0' | dd of="$obj_file" bs=1 seek="$2"

Some other tests such as t4205 also rely on "printf" being
binary-safe.  Phew.

> +}
> +
> +test_expect_success 'setup corrupt repo' '
> +	git init bit-error &&
> +	(
> +		cd bit-error &&
> +		test_commit content &&
> +		corrupt_byte HEAD:content.t 10
> +	)
> +'
> +
> +test_expect_success 'streaming a corrupt blob fails' '

"fails gracefully", maybe, to be more precise.

With or without the two changes suggested above,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>