Re: [PATCH 1/3] ewah_read_mmap: bounds-check mmap reads

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jun 16, 2018, 14:35 UTC
Message-ID: <20180616143513.10086-1-szeder.dev@gmail.com>
In-Reply-To: <20180615033112.GA20390@sigill.intra.peff.net>

Show 16 quoted lines

> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
> index 423c0a475f..237ee6e5fc 100755
> --- a/t/t5310-pack-bitmaps.sh
> +++ b/t/t5310-pack-bitmaps.sh
> @@ -331,4 +331,17 @@ test_expect_success 'pack reuse respects --incremental' '
>  	git show-index <empty.idx >actual &&
>  	test_cmp expect actual
>  '
> +
> +test_expect_success 'truncated bitmap fails gracefully' '
> +	git repack -ad &&
> +	git rev-list --use-bitmap-index --count --all >expect &&
> +	bitmap=$(ls .git/objects/pack/*.bitmap) &&
> +	test_when_finished "rm -f $bitmap" &&
> +	head -c 512 <$bitmap >$bitmap.tmp &&
> +	mv $bitmap.tmp $bitmap &&

This line turns out to be problematic on OSX and ultimately causes the test to fail.

When OSX's 'mv's destination is read-only, it asks whether to replace the destination even though in the test its stdin is not a terminal (and thus doesn't conform to POSIX[1]). Since the '.bitmap' file is read-only, and since 'mv' obviously doesn't get an affirmative response from /dev/null, the original '.bitmap' file is not overwritten, the subsequent 'git rev-list' doesn't print any error message, and finally 'test_i18ngrep' causes the test to fail.

The relevant part of the '-x' test output on Travis CI:

  ++mv .git/objects/pack/pack-8886db3fce4f9657c1a43fee7d3ea4f2a4b5be2d.bitmap.tmp .git/objects/pack/pack-8886db3fce4f9657c1a43fee7d3ea4f2a4b5be2d.bitmap
  override r--r--r--  travis/staff for .git/objects/pack/pack-8886db3fce4f9657c1a43fee7d3ea4f2a4b5be2d.bitmap? (y/n [n]) not overwritten
  ++git rev-list --use-bitmap-index --count --all
  ++test_cmp expect actual
  ++diff -u expect actual
  ++test_i18ngrep corrupt stderr
  ++eval 'last_arg=${2}'
  +++last_arg=stderr
  ++test -f stderr
  ++test 2 -lt 2
  ++test 'x!' = xcorrupt
  ++test -n ''
  ++test 'x!' = xcorrupt
  ++grep corrupt stderr
  ++echo 'error: '\''grep corrupt' 'stderr'\'' didn'\''t find a match in:'
  error: 'grep corrupt stderr' didn't find a match in:
  ++test -s stderr
  ++echo '<File '\''stderr'\'' is empty>'
  <File 'stderr' is empty>
  ++return 1
  error: last command exited with $?=1
  not ok 43 - truncated bitmap fails gracefully

As far as I can tell, 'mv -f' appears to make the test work on OSX as well.

I've run a build job with an additional 'grep ^override t/test-results/*.out' command following the tests to see whether there are any other cases where OSX 'mv' doesn't overwrite a read-only file without causing the tests to fail, but found nothing. (But note that the OSX build jobs don't run all tests.)

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/mv.html

Show 5 quoted lines

> +	git rev-list --use-bitmap-index --count --all >actual 2>stderr &&
> +	test_cmp expect actual &&
> +	test_i18ngrep corrupt stderr
> +'
> +