On Mon, Aug 5, 2013 at 3:29 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > >> A bounds checking bug allows the X in -LX to extend one line past the >> end of file. For example, given a file with 5 lines, -L6 is accepted as >> valid. Demonstrate this problem. >> >> While here, also add tests to check that the remaining cases of X and Y >> in -LX,Y are handled correctly at and in the vicinity of end-of-file. >> >> Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx> >> --- >> t/annotate-tests.sh | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh >> index 3524eaf..02fbbf1 100644 >> --- a/t/annotate-tests.sh >> +++ b/t/annotate-tests.sh >> @@ -225,10 +225,32 @@ test_expect_success 'blame -L /RE/,-N' ' >> check_count -L/99/,-3 B 1 B2 1 D 1 >> ' >> >> +# 'file' ends with an incomplete line, so 'wc' reports one fewer lines than >> +# git-blame sees, hence the last line is actually $(wc...)+1. >> +test_expect_success 'blame -L X (X == nlines)' ' >> + n=$(expr $(wc -l <file) + 1) && >> + check_count -L$n C 1 >> +' >> + >> +test_expect_failure 'blame -L X (X == nlines + 1)' ' >> + n=$(expr $(wc -l <file) + 2) && >> + test_must_fail $PROG -L$n file >> +' >> + >> test_expect_success 'blame -L X (X > nlines)' ' >> test_must_fail $PROG -L12345 file >> ' >> +test_expect_success 'blame -L ,Y (Y == nlines)' ' >> + n=$(expr $(wc -l <file) + 1) && >> + check_count -L,$n A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1 >> +' >> + >> +test_expect_success 'blame -L ,Y (Y == nlines + 1)' ' >> + n=$(expr $(wc -l <file) + 2) && >> + test_must_fail $PROG -L,$n file >> +' >> + > > This is somewhat curious. > > Does the problem triggers only on a file that ends with an > incomplete line, or the test was inserted at this location only > because it was convenient and the problem exists with or without the > incomplete final line? > > I am guessing that it is the latter. The problem exists with and without the incomplete line. The comment mentioning "incomplete line" and "wc" was inserted to explain why it was necessary to add one to the result of wc, which might not otherwise be obvious. The tests were inserted at this location because they are semantically related to the "blame -L ,Y (Y > nlines)" test which already exists in the file (quote just below this response). >> test_expect_success 'blame -L ,Y (Y > nlines)' ' >> test_must_fail $PROG -L,12345 file >> ' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html