log -L: fix overlapping input ranges

The existing code was too defensive, and would trigger the assert in range_set_append() if the user gave overlapping ranges. The intent was always to define overlapping ranges as just the union of all of them, as evidenced by the call to sort_and_merge_range_set(). (Which was already used, unlike what the comment said.) Fix by splitting out the meat of range_set_append() to a new _unsafe() function that lacks the paranoia. sort_and_merge_range_set will fix up the ranges, so we don't need the checks there. Signed-off-by: Thomas Rast <trast@inf.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Thomas Rast committed Apr 5, 2013 at 16:34 UTC 209618860c2627a4e134a15472587c574b328b40
5 files changed +366 -7
line-log.c
+10 -7
@@ -56,16 +56,21 @@ static void range_set_move(struct range_set *dst, struct range_set *src)
56 }
57
58 /* tack on a _new_ range _at the end_ */
59 -static void range_set_append(struct range_set *rs, long a, long b)
59 +static void range_set_append_unsafe(struct range_set *rs, long a, long b)
60 {
61 assert(a <= b);
62 - assert(rs->nr == 0 || rs->ranges[rs->nr-1].end <= a);
62 range_set_grow(rs, 1);
63 rs->ranges[rs->nr].start = a;
64 rs->ranges[rs->nr].end = b;
65 rs->nr++;
66 }
67
68 +static void range_set_append(struct range_set *rs, long a, long b)
69 +{
70 + assert(rs->nr == 0 || rs->ranges[rs->nr-1].end <= a);
71 + range_set_append_unsafe(rs, a, b);
72 +}
73 +
74 static int range_cmp(const void *_r, const void *_s)
75 {
76 const struct range *r = _r;
@@ -99,10 +104,8 @@ static void range_set_check_invariants(struct range_set *rs)
104 }
105
106 /*
102 - * Helper: In-place pass of sorting and merging the ranges in the
103 - * range set, to re-establish the invariants after another operation
104 - *
105 - * NEEDSWORK currently not needed
107 + * In-place pass of sorting and merging the ranges in the range set,
108 + * to establish the invariants when we get the ranges from the user
109 */
110 static void sort_and_merge_range_set(struct range_set *rs)
111 {
@@ -280,7 +283,7 @@ static void line_log_data_insert(struct line_log_data **list,
283 struct line_log_data *p = search_line_log_data(*list, spec->path, &ip);
284
285 if (p) {
283 - range_set_append(&p->ranges, begin, end);
286 + range_set_append_unsafe(&p->ranges, begin, end);
287 sort_and_merge_range_set(&p->ranges);
288 free_filespec(spec);
289 return;
t/t4211-line-log.sh
+6
@@ -39,6 +39,12 @@ canned_test "-L 24,+1:a.c simple" vanishes-early
39
40 canned_test "-L '/long f/,/^}/:b.c' move-support" move-support-f
41
42 +canned_test "-L 4,12:a.c -L :main:a.c simple" multiple
43 +canned_test "-L 4,18:a.c -L :main:a.c simple" multiple-overlapping
44 +canned_test "-L :main:a.c -L 4,18:a.c simple" multiple-overlapping
45 +canned_test "-L 4:a.c -L 8,12:a.c simple" multiple-superset
46 +canned_test "-L 8,12:a.c -L 4:a.c simple" multiple-superset
47 +
48 test_bad_opts "-L" "switch.*requires a value"
49 test_bad_opts "-L b.c" "argument.*not of the form"
50 test_bad_opts "-L 1:" "argument.*not of the form"
t/t4211/expect.multiple new
+104
@@ -0,0 +1,104 @@
1 +commit 4659538844daa2849b1a9e7d6fadb96fcd26fc83
2 +Author: Thomas Rast <trast@student.ethz.ch>
3 +Date: Thu Feb 28 10:48:43 2013 +0100
4 +
5 + change back to complete line
6 +
7 +diff --git a/a.c b/a.c
8 +--- a/a.c
9 ++++ b/a.c
10 +@@ -18,5 +18,7 @@
11 + int main ()
12 + {
13 + printf("%ld\n", f(15));
14 + return 0;
15 +-}
16 +\ No newline at end of file
17 ++}
18 ++
19 ++/* incomplete lines are bad! */
20 +
21 +commit 100b61a6f2f720f812620a9d10afb3a960ccb73c
22 +Author: Thomas Rast <trast@student.ethz.ch>
23 +Date: Thu Feb 28 10:48:10 2013 +0100
24 +
25 + change to an incomplete line at end
26 +
27 +diff --git a/a.c b/a.c
28 +--- a/a.c
29 ++++ b/a.c
30 +@@ -18,5 +18,5 @@
31 + int main ()
32 + {
33 + printf("%ld\n", f(15));
34 + return 0;
35 +-}
36 ++}
37 +\ No newline at end of file
38 +
39 +commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2
40 +Author: Thomas Rast <trast@student.ethz.ch>
41 +Date: Thu Feb 28 10:45:16 2013 +0100
42 +
43 + touch both functions
44 +
45 +diff --git a/a.c b/a.c
46 +--- a/a.c
47 ++++ b/a.c
48 +@@ -3,9 +3,9 @@
49 +-int f(int x)
50 ++long f(long x)
51 + {
52 + int s = 0;
53 + while (x) {
54 + x >>= 1;
55 + s++;
56 + }
57 + return s;
58 + }
59 +@@ -17,5 +17,5 @@
60 + int main ()
61 + {
62 +- printf("%d\n", f(15));
63 ++ printf("%ld\n", f(15));
64 + return 0;
65 + }
66 +
67 +commit f04fb20f2c77850996cba739709acc6faecc58f7
68 +Author: Thomas Rast <trast@student.ethz.ch>
69 +Date: Thu Feb 28 10:44:55 2013 +0100
70 +
71 + change f()
72 +
73 +diff --git a/a.c b/a.c
74 +--- a/a.c
75 ++++ b/a.c
76 +@@ -3,8 +3,9 @@
77 + int f(int x)
78 + {
79 + int s = 0;
80 + while (x) {
81 + x >>= 1;
82 + s++;
83 + }
84 ++ return s;
85 + }
86 +
87 +commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a
88 +Author: Thomas Rast <trast@student.ethz.ch>
89 +Date: Thu Feb 28 10:44:48 2013 +0100
90 +
91 + initial
92 +
93 +diff --git a/a.c b/a.c
94 +--- /dev/null
95 ++++ b/a.c
96 +@@ -0,0 +3,8 @@
97 ++int f(int x)
98 ++{
99 ++ int s = 0;
100 ++ while (x) {
101 ++ x >>= 1;
102 ++ s++;
103 ++ }
104 ++}
t/t4211/expect.multiple-overlapping new
+187
@@ -0,0 +1,187 @@
1 +commit 4659538844daa2849b1a9e7d6fadb96fcd26fc83
2 +Author: Thomas Rast <trast@student.ethz.ch>
3 +Date: Thu Feb 28 10:48:43 2013 +0100
4 +
5 + change back to complete line
6 +
7 +diff --git a/a.c b/a.c
8 +--- a/a.c
9 ++++ b/a.c
10 +@@ -4,19 +4,21 @@
11 + long f(long x)
12 + {
13 + int s = 0;
14 + while (x) {
15 + x >>= 1;
16 + s++;
17 + }
18 + return s;
19 + }
20 +
21 + /*
22 + * This is only an example!
23 + */
24 +
25 + int main ()
26 + {
27 + printf("%ld\n", f(15));
28 + return 0;
29 +-}
30 +\ No newline at end of file
31 ++}
32 ++
33 ++/* incomplete lines are bad! */
34 +
35 +commit 100b61a6f2f720f812620a9d10afb3a960ccb73c
36 +Author: Thomas Rast <trast@student.ethz.ch>
37 +Date: Thu Feb 28 10:48:10 2013 +0100
38 +
39 + change to an incomplete line at end
40 +
41 +diff --git a/a.c b/a.c
42 +--- a/a.c
43 ++++ b/a.c
44 +@@ -4,19 +4,19 @@
45 + long f(long x)
46 + {
47 + int s = 0;
48 + while (x) {
49 + x >>= 1;
50 + s++;
51 + }
52 + return s;
53 + }
54 +
55 + /*
56 + * This is only an example!
57 + */
58 +
59 + int main ()
60 + {
61 + printf("%ld\n", f(15));
62 + return 0;
63 +-}
64 ++}
65 +\ No newline at end of file
66 +
67 +commit 39b6eb2d5b706d3322184a169f666f25ed3fbd00
68 +Author: Thomas Rast <trast@student.ethz.ch>
69 +Date: Thu Feb 28 10:45:41 2013 +0100
70 +
71 + touch comment
72 +
73 +diff --git a/a.c b/a.c
74 +--- a/a.c
75 ++++ b/a.c
76 +@@ -3,19 +3,19 @@
77 + long f(long x)
78 + {
79 + int s = 0;
80 + while (x) {
81 + x >>= 1;
82 + s++;
83 + }
84 + return s;
85 + }
86 +
87 + /*
88 +- * A comment.
89 ++ * This is only an example!
90 + */
91 +
92 + int main ()
93 + {
94 + printf("%ld\n", f(15));
95 + return 0;
96 + }
97 +
98 +commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2
99 +Author: Thomas Rast <trast@student.ethz.ch>
100 +Date: Thu Feb 28 10:45:16 2013 +0100
101 +
102 + touch both functions
103 +
104 +diff --git a/a.c b/a.c
105 +--- a/a.c
106 ++++ b/a.c
107 +@@ -3,19 +3,19 @@
108 +-int f(int x)
109 ++long f(long x)
110 + {
111 + int s = 0;
112 + while (x) {
113 + x >>= 1;
114 + s++;
115 + }
116 + return s;
117 + }
118 +
119 + /*
120 + * A comment.
121 + */
122 +
123 + int main ()
124 + {
125 +- printf("%d\n", f(15));
126 ++ printf("%ld\n", f(15));
127 + return 0;
128 + }
129 +
130 +commit f04fb20f2c77850996cba739709acc6faecc58f7
131 +Author: Thomas Rast <trast@student.ethz.ch>
132 +Date: Thu Feb 28 10:44:55 2013 +0100
133 +
134 + change f()
135 +
136 +diff --git a/a.c b/a.c
137 +--- a/a.c
138 ++++ b/a.c
139 +@@ -3,18 +3,19 @@
140 + int f(int x)
141 + {
142 + int s = 0;
143 + while (x) {
144 + x >>= 1;
145 + s++;
146 + }
147 ++ return s;
148 + }
149 +
150 + /*
151 + * A comment.
152 + */
153 +
154 + int main ()
155 + {
156 + printf("%d\n", f(15));
157 + return 0;
158 + }
159 +
160 +commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a
161 +Author: Thomas Rast <trast@student.ethz.ch>
162 +Date: Thu Feb 28 10:44:48 2013 +0100
163 +
164 + initial
165 +
166 +diff --git a/a.c b/a.c
167 +--- /dev/null
168 ++++ b/a.c
169 +@@ -0,0 +3,18 @@
170 ++int f(int x)
171 ++{
172 ++ int s = 0;
173 ++ while (x) {
174 ++ x >>= 1;
175 ++ s++;
176 ++ }
177 ++}
178 ++
179 ++/*
180 ++ * A comment.
181 ++ */
182 ++
183 ++int main ()
184 ++{
185 ++ printf("%d\n", f(15));
186 ++ return 0;
187 ++}
t/t4211/expect.multiple-superset new
+59
@@ -0,0 +1,59 @@
1 +commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2
2 +Author: Thomas Rast <trast@student.ethz.ch>
3 +Date: Thu Feb 28 10:45:16 2013 +0100
4 +
5 + touch both functions
6 +
7 +diff --git a/a.c b/a.c
8 +--- a/a.c
9 ++++ b/a.c
10 +@@ -3,9 +3,9 @@
11 +-int f(int x)
12 ++long f(long x)
13 + {
14 + int s = 0;
15 + while (x) {
16 + x >>= 1;
17 + s++;
18 + }
19 + return s;
20 + }
21 +
22 +commit f04fb20f2c77850996cba739709acc6faecc58f7
23 +Author: Thomas Rast <trast@student.ethz.ch>
24 +Date: Thu Feb 28 10:44:55 2013 +0100
25 +
26 + change f()
27 +
28 +diff --git a/a.c b/a.c
29 +--- a/a.c
30 ++++ b/a.c
31 +@@ -3,8 +3,9 @@
32 + int f(int x)
33 + {
34 + int s = 0;
35 + while (x) {
36 + x >>= 1;
37 + s++;
38 + }
39 ++ return s;
40 + }
41 +
42 +commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a
43 +Author: Thomas Rast <trast@student.ethz.ch>
44 +Date: Thu Feb 28 10:44:48 2013 +0100
45 +
46 + initial
47 +
48 +diff --git a/a.c b/a.c
49 +--- /dev/null
50 ++++ b/a.c
51 +@@ -0,0 +3,8 @@
52 ++int f(int x)
53 ++{
54 ++ int s = 0;
55 ++ while (x) {
56 ++ x >>= 1;
57 ++ s++;
58 ++ }
59 ++}