From b6679e768f39f718b7f2983d1958f5fb1121f356 Mon Sep 17 00:00:00 2001 From: Eric Sunshine <sunshine@sunshineco.com> Date: Tue, 23 Jul 2013 10:28:04 -0400 Subject: [PATCH 1/6] range-set: fix sort_and_merge_range_set() corner case bug When handed an empty range_set (range_set.nr == 0), sort_and_merge_range_set() incorrectly sets range_set.nr to 1 at exit. Subsequent range_set functions then access the bogus range at element zero and crash or throw an assertion failure. Fix this bug. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Acked-by: Thomas Rast <trast@inf.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- line-log.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/line-log.c b/line-log.c index 8cc29a0000..52348796c1 100644 --- a/line-log.c +++ b/line-log.c @@ -110,12 +110,12 @@ static void range_set_check_invariants(struct range_set *rs) static void sort_and_merge_range_set(struct range_set *rs) { int i; - int o = 1; /* output cursor */ + int o = 0; /* output cursor */ qsort(rs->ranges, rs->nr, sizeof(struct range), range_cmp); - for (i = 1; i < rs->nr; i++) { - if (rs->ranges[i].start <= rs->ranges[o-1].end) { + for (i = 0; i < rs->nr; i++) { + if (o > 0 && rs->ranges[i].start <= rs->ranges[o-1].end) { if (rs->ranges[o-1].end < rs->ranges[i].end) rs->ranges[o-1].end = rs->ranges[i].end; } else { From 58960978462e66c3dbc53b274988e7f27e7c74e2 Mon Sep 17 00:00:00 2001 From: Eric Sunshine <sunshine@sunshineco.com> Date: Tue, 23 Jul 2013 10:28:05 -0400 Subject: [PATCH 2/6] t4211: demonstrate empty -L range crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Acked-by: Thomas Rast <trast@inf.ethz.ch> Helped-by: SZEDER Gábor <szeder@ira.uka.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t4211-line-log.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 7776f93e3d..9042178124 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -64,4 +64,12 @@ test_bad_opts "-L 1,1000:b.c" "has only.*lines" test_bad_opts "-L :b.c" "argument.*not of the form" test_bad_opts "-L :foo:b.c" "no match" +# There is a separate bug when an empty -L range is the first -L encountered, +# thus to demonstrate this particular bug, the empty -L range must follow a +# non-empty -L range. +test_expect_failure '-L {empty-range} (any -L)' ' + n=$(expr $(wc -l <b.c) + 1) && + git log -L1,1:b.c -L$n:b.c +' + test_done From 99780b0a4ab22c903516601bb3634e6477d7f1c1 Mon Sep 17 00:00:00 2001 From: Eric Sunshine <sunshine@sunshineco.com> Date: Tue, 23 Jul 2013 10:28:07 -0400 Subject: [PATCH 3/6] t4211: demonstrate crash when first -L encountered is empty range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Acked-by: Thomas Rast <trast@inf.ethz.ch> Helped-by: SZEDER Gábor <szeder@ira.uka.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t4211-line-log.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 9042178124..d98efb3c4e 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -72,4 +72,9 @@ test_expect_failure '-L {empty-range} (any -L)' ' git log -L1,1:b.c -L$n:b.c ' +test_expect_failure '-L {empty-range} (first -L)' ' + n=$(expr $(wc -l <b.c) + 1) && + git log -L$n:b.c +' + test_done From f8395edc6f5c59a92adcf42ea20a01872ec22700 Mon Sep 17 00:00:00 2001 From: Eric Sunshine <sunshine@sunshineco.com> Date: Tue, 23 Jul 2013 10:28:06 -0400 Subject: [PATCH 4/6] range-set: satisfy non-empty ranges invariant range-set invariants are: ranges must be (1) non-empty, (2) disjoint, (3) sorted in ascending order. During processing, various range-set utility functions break the invariants (for instance, by adding empty ranges), with the expectation that a finalizing sort_and_merge_range_set() will restore sanity. sort_and_merge_range_set(), however, neglects to fold out empty ranges, thus it fails to satisfy the non-empty constraint. Subsequent range-set functions crash or throw an assertion failure upon encountering such an anomaly. Rectify the situation by having sort_and_merge_range_set() fold out empty ranges. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Acked-by: Thomas Rast <trast@inf.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- line-log.c | 2 ++ t/t4211-line-log.sh | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/line-log.c b/line-log.c index 52348796c1..6f94d56063 100644 --- a/line-log.c +++ b/line-log.c @@ -115,6 +115,8 @@ static void sort_and_merge_range_set(struct range_set *rs) qsort(rs->ranges, rs->nr, sizeof(struct range), range_cmp); for (i = 0; i < rs->nr; i++) { + if (rs->ranges[i].start == rs->ranges[i].end) + continue; if (o > 0 && rs->ranges[i].start <= rs->ranges[o-1].end) { if (rs->ranges[o-1].end < rs->ranges[i].end) rs->ranges[o-1].end = rs->ranges[i].end; diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index d98efb3c4e..e7a6e49965 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -67,7 +67,8 @@ test_bad_opts "-L :foo:b.c" "no match" # There is a separate bug when an empty -L range is the first -L encountered, # thus to demonstrate this particular bug, the empty -L range must follow a # non-empty -L range. -test_expect_failure '-L {empty-range} (any -L)' ' +test_expect_success '-L {empty-range} (any -L)' ' + n=$(expr $(cat b.c | wc -l) + 1) && n=$(expr $(wc -l <b.c) + 1) && git log -L1,1:b.c -L$n:b.c ' From df6308eb82bc9d2074b35387583548b8b153433d Mon Sep 17 00:00:00 2001 From: Eric Sunshine <sunshine@sunshineco.com> Date: Tue, 23 Jul 2013 10:28:08 -0400 Subject: [PATCH 5/6] line-log: fix "log -LN" crash when N is last line of file range-set invariants are: ranges must be (1) non-empty, (2) disjoint, (3) sorted in ascending order. line_log_data_insert() breaks the non-empty invariant under the following conditions: the incoming range is empty and the pathname attached to the range has not yet been encountered. In this case, line_log_data_insert() assigns the empty range to a new line_log_data record without taking any action to ensure that the empty range is eventually folded out. Subsequent range-set functions crash or throw an assertion failure upon encountering such an anomaly. Fix this bug. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Acked-by: Thomas Rast <trast@inf.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- line-log.c | 1 + t/t4211-line-log.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/line-log.c b/line-log.c index 6f94d56063..c2d01dccc2 100644 --- a/line-log.c +++ b/line-log.c @@ -299,6 +299,7 @@ static void line_log_data_insert(struct line_log_data **list, p = xcalloc(1, sizeof(struct line_log_data)); p->path = path; range_set_append(&p->ranges, begin, end); + sort_and_merge_range_set(&p->ranges); if (ip) { p->next = ip->next; ip->next = p; diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index e7a6e49965..00a850d611 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -73,7 +73,7 @@ test_expect_success '-L {empty-range} (any -L)' ' git log -L1,1:b.c -L$n:b.c ' -test_expect_failure '-L {empty-range} (first -L)' ' +test_expect_success '-L {empty-range} (first -L)' ' n=$(expr $(wc -l <b.c) + 1) && git log -L$n:b.c ' From d3a486c47d920efc183645766d762fd697c42088 Mon Sep 17 00:00:00 2001 From: Junio C Hamano <gitster@pobox.com> Date: Wed, 24 Jul 2013 07:53:25 -0700 Subject: [PATCH 6/6] t4211: fix incorrect rebase at f8395edc (range-set: satisfy non-empty ranges invariant) Wnen I rewrote "cat b.c | wc -l" into "wc -l <b.c" to squash in a suggestion on the list to this series, I screwed up subsequent rebase. Fix it up. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t4211-line-log.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 00a850d611..7665d6785c 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -68,7 +68,6 @@ test_bad_opts "-L :foo:b.c" "no match" # thus to demonstrate this particular bug, the empty -L range must follow a # non-empty -L range. test_expect_success '-L {empty-range} (any -L)' ' - n=$(expr $(cat b.c | wc -l) + 1) && n=$(expr $(wc -l <b.c) + 1) && git log -L1,1:b.c -L$n:b.c '