Skip to content

Commit 118b9d5

Browse files
gitstergitster
committed
Merge branch 'es/blame-L-more'
More fixes to the code to parse the "-L" option in "log" and "blame". * es/blame-L-more: blame: reject empty ranges -L,+0 and -L,-0 t8001/t8002: blame: demonstrate acceptance of bogus -L,+0 and -L,-0 blame: reject empty ranges -LX,+0 and -LX,-0 t8001/t8002: blame: demonstrate acceptance of bogus -LX,+0 and -LX,-0 log: fix -L bounds checking bug t4211: retire soon-to-be unimplementable tests t4211: log: demonstrate -L bounds checking bug blame: fix -L bounds checking bug t8001/t8002: blame: add empty file & partial-line tests t8001/t8002: blame: demonstrate -L bounds checking bug t8001/t8002: blame: decompose overly-large test
2 parents 4301262 + 5d57cac commit 118b9d5

File tree

5 files changed

+166
-19
lines changed

5 files changed

+166
-19
lines changed

‎builtin/blame.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2495,13 +2495,13 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
24952495
bottom = top = 0;
24962496
if (bottomtop)
24972497
prepare_blame_range(&sb, bottomtop, lno, &bottom, &top);
2498+
if (lno < top || ((lno || bottom) && lno < bottom))
2499+
die("file %s has only %lu lines", path, lno);
24982500
if (bottom < 1)
24992501
bottom = 1;
25002502
if (top < 1)
25012503
top = lno;
25022504
bottom--;
2503-
if (lno < top || lno < bottom)
2504-
die("file %s has only %lu lines", path, lno);
25052505

25062506
ent = xcalloc(1, sizeof(*ent));
25072507
ent->lno = bottom;

‎line-log.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -594,13 +594,13 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args)
594594
lines, &begin, &end,
595595
full_name))
596596
die("malformed -L argument '%s'", range_part);
597+
if (lines < end || ((lines || begin) && lines < begin))
598+
die("file %s has only %lu lines", name_part, lines);
597599
if (begin < 1)
598600
begin = 1;
599601
if (end < 1)
600602
end = lines;
601603
begin--;
602-
if (lines < end || lines < begin)
603-
die("file %s has only %ld lines", name_part, lines);
604604
line_log_data_insert(&ranges, full_name, begin, end);
605605

606606
free_filespec(spec);

‎line-range.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
2121
* for 20 lines, or "-L <something>,-5" for 5 lines ending at
2222
* <something>.
2323
*/
24-
if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
24+
if (1 <= begin && (spec[0] == '+' || spec[0] == '-')) {
2525
num = strtol(spec + 1, &term, 10);
2626
if (term != spec + 1) {
2727
if (!ret)
2828
return term;
29+
if (num == 0)
30+
die("-L invalid empty range");
2931
if (spec[0] == '-')
3032
num = 0 - num;
3133
if (0 < num)

‎t/annotate-tests.sh

Lines changed: 135 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,14 @@ test_expect_success 'blame -L Y,X (undocumented)' '
185185
check_count -L6,3 B 1 B1 1 B2 1 D 1
186186
'
187187

188+
test_expect_success 'blame -L ,+0' '
189+
test_must_fail $PROG -L,+0 file
190+
'
191+
192+
test_expect_success 'blame -L X,+0' '
193+
test_must_fail $PROG -L1,+0 file
194+
'
195+
188196
test_expect_success 'blame -L X,+1' '
189197
check_count -L3,+1 B2 1
190198
'
@@ -193,6 +201,14 @@ test_expect_success 'blame -L X,+N' '
193201
check_count -L3,+4 B 1 B1 1 B2 1 D 1
194202
'
195203

204+
test_expect_success 'blame -L ,-0' '
205+
test_must_fail $PROG -L,-0 file
206+
'
207+
208+
test_expect_success 'blame -L X,-0' '
209+
test_must_fail $PROG -L1,-0 file
210+
'
211+
196212
test_expect_success 'blame -L X,-1' '
197213
check_count -L3,-1 B2 1
198214
'
@@ -225,10 +241,32 @@ test_expect_success 'blame -L /RE/,-N' '
225241
check_count -L/99/,-3 B 1 B2 1 D 1
226242
'
227243

244+
# 'file' ends with an incomplete line, so 'wc' reports one fewer lines than
245+
# git-blame sees, hence the last line is actually $(wc...)+1.
246+
test_expect_success 'blame -L X (X == nlines)' '
247+
n=$(expr $(wc -l <file) + 1) &&
248+
check_count -L$n C 1
249+
'
250+
251+
test_expect_success 'blame -L X (X == nlines + 1)' '
252+
n=$(expr $(wc -l <file) + 2) &&
253+
test_must_fail $PROG -L$n file
254+
'
255+
228256
test_expect_success 'blame -L X (X > nlines)' '
229257
test_must_fail $PROG -L12345 file
230258
'
231259

260+
test_expect_success 'blame -L ,Y (Y == nlines)' '
261+
n=$(expr $(wc -l <file) + 1) &&
262+
check_count -L,$n A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1 E 1
263+
'
264+
265+
test_expect_success 'blame -L ,Y (Y == nlines + 1)' '
266+
n=$(expr $(wc -l <file) + 2) &&
267+
test_must_fail $PROG -L,$n file
268+
'
269+
232270
test_expect_success 'blame -L ,Y (Y > nlines)' '
233271
test_must_fail $PROG -L,12345 file
234272
'
@@ -275,12 +313,102 @@ test_expect_success 'blame -L :nomatch' '
275313
test_must_fail $PROG -L:nomatch hello.c
276314
'
277315

278-
test_expect_success 'blame -L bogus' '
279-
test_must_fail $PROG -L file &&
280-
test_must_fail $PROG -L1,+ file &&
281-
test_must_fail $PROG -L1,- file &&
282-
test_must_fail $PROG -LX file &&
283-
test_must_fail $PROG -L1,X file &&
284-
test_must_fail $PROG -L1,+N file &&
316+
test_expect_success 'setup incremental' '
317+
(
318+
GIT_AUTHOR_NAME=I &&
319+
export GIT_AUTHOR_NAME &&
320+
[email protected] &&
321+
export GIT_AUTHOR_EMAIL &&
322+
>incremental &&
323+
git add incremental &&
324+
git commit -m "step 0" &&
325+
printf "partial" >>incremental &&
326+
git commit -a -m "step 0.5" &&
327+
echo >>incremental &&
328+
git commit -a -m "step 1"
329+
)
330+
'
331+
332+
test_expect_success 'blame empty' '
333+
check_count -h HEAD^^ -f incremental
334+
'
335+
336+
test_expect_success 'blame -L 0 empty (undocumented)' '
337+
check_count -h HEAD^^ -f incremental -L0
338+
'
339+
340+
test_expect_success 'blame -L 1 empty' '
341+
test_must_fail $PROG -L1 incremental HEAD^^
342+
'
343+
344+
test_expect_success 'blame -L 2 empty' '
345+
test_must_fail $PROG -L2 incremental HEAD^^
346+
'
347+
348+
test_expect_success 'blame half' '
349+
check_count -h HEAD^ -f incremental I 1
350+
'
351+
352+
test_expect_success 'blame -L 0 half (undocumented)' '
353+
check_count -h HEAD^ -f incremental -L0 I 1
354+
'
355+
356+
test_expect_success 'blame -L 1 half' '
357+
check_count -h HEAD^ -f incremental -L1 I 1
358+
'
359+
360+
test_expect_success 'blame -L 2 half' '
361+
test_must_fail $PROG -L2 incremental HEAD^
362+
'
363+
364+
test_expect_success 'blame -L 3 half' '
365+
test_must_fail $PROG -L3 incremental HEAD^
366+
'
367+
368+
test_expect_success 'blame full' '
369+
check_count -f incremental I 1
370+
'
371+
372+
test_expect_success 'blame -L 0 full (undocumented)' '
373+
check_count -f incremental -L0 I 1
374+
'
375+
376+
test_expect_success 'blame -L 1 full' '
377+
check_count -f incremental -L1 I 1
378+
'
379+
380+
test_expect_success 'blame -L 2 full' '
381+
test_must_fail $PROG -L2 incremental
382+
'
383+
384+
test_expect_success 'blame -L 3 full' '
385+
test_must_fail $PROG -L3 incremental
386+
'
387+
388+
test_expect_success 'blame -L' '
389+
test_must_fail $PROG -L file
390+
'
391+
392+
test_expect_success 'blame -L X,+' '
393+
test_must_fail $PROG -L1,+ file
394+
'
395+
396+
test_expect_success 'blame -L X,-' '
397+
test_must_fail $PROG -L1,- file
398+
'
399+
400+
test_expect_success 'blame -L X (non-numeric X)' '
401+
test_must_fail $PROG -LX file
402+
'
403+
404+
test_expect_success 'blame -L X,Y (non-numeric Y)' '
405+
test_must_fail $PROG -L1,Y file
406+
'
407+
408+
test_expect_success 'blame -L X,+N (non-numeric N)' '
409+
test_must_fail $PROG -L1,+N file
410+
'
411+
412+
test_expect_success 'blame -L X,-N (non-numeric N)' '
285413
test_must_fail $PROG -L1,-N file
286414
'

‎t/t4211-line-log.sh

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,34 @@ test_bad_opts "-L 1,1000:b.c" "has only.*lines"
6464
test_bad_opts "-L :b.c" "argument.*not of the form"
6565
test_bad_opts "-L :foo:b.c" "no match"
6666

67-
# There is a separate bug when an empty -L range is the first -L encountered,
68-
# thus to demonstrate this particular bug, the empty -L range must follow a
69-
# non-empty -L range.
70-
test_expect_success '-L {empty-range} (any -L)' '
67+
test_expect_success '-L X (X == nlines)' '
68+
n=$(wc -l <b.c) &&
69+
git log -L $n:b.c
70+
'
71+
72+
test_expect_success '-L X (X == nlines + 1)' '
7173
n=$(expr $(wc -l <b.c) + 1) &&
72-
git log -L1,1:b.c -L$n:b.c
74+
test_must_fail git log -L $n:b.c
75+
'
76+
77+
test_expect_success '-L X (X == nlines + 2)' '
78+
n=$(expr $(wc -l <b.c) + 2) &&
79+
test_must_fail git log -L $n:b.c
7380
'
7481

75-
test_expect_success '-L {empty-range} (first -L)' '
82+
test_expect_success '-L ,Y (Y == nlines)' '
83+
n=$(printf "%d" $(wc -l <b.c)) &&
84+
git log -L ,$n:b.c
85+
'
86+
87+
test_expect_success '-L ,Y (Y == nlines + 1)' '
7688
n=$(expr $(wc -l <b.c) + 1) &&
77-
git log -L$n:b.c
89+
test_must_fail git log -L ,$n:b.c
90+
'
91+
92+
test_expect_success '-L ,Y (Y == nlines + 2)' '
93+
n=$(expr $(wc -l <b.c) + 2) &&
94+
test_must_fail git log -L ,$n:b.c
7895
'
7996

8097
test_done

0 commit comments

Comments
 (0)