apply: fix leaking string in `match_fragment()`
Before calling `update_pre_post_images()`, we call `strbuf_detach()` to put its buffer into a new string variable that we then pass to that function. Besides being rather pointless, it also causes us to leak memory of that variable because we never free it. Get rid of the variable altogether and instead reach into the `strbuf` directly. While at it, refactor the code to have a common exit path and mark string that do not contain allocated memory as constant. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Jun 11, 2024 at 11:20 UTC
4806c55c86f7cc45f60a7ff5d757874072265deb
2 files changed
+56
-32
apply.c
+55
-32
@@ -2494,18 +2494,21 @@ static int match_fragment(struct apply_state *state,
2494
int match_beginning, int match_end)
2495
{
2496
int i;
2497
- char *fixed_buf, *buf, *orig, *target;
2498
- struct strbuf fixed;
2499
- size_t fixed_len, postlen;
2497
+ const char *orig, *target;
2498
+ struct strbuf fixed = STRBUF_INIT;
2499
+ size_t postlen;
2500
int preimage_limit;
2501
+ int ret;
2502
2503
if (preimage->nr + current_lno <= img->nr) {
2504
/*
2505
* The hunk falls within the boundaries of img.
2506
*/
2507
preimage_limit = preimage->nr;
2507
- if (match_end && (preimage->nr + current_lno != img->nr))
2508
- return 0;
2508
+ if (match_end && (preimage->nr + current_lno != img->nr)) {
2509
+ ret = 0;
2510
+ goto out;
2511
+ }
2512
} else if (state->ws_error_action == correct_ws_error &&
2513
(ws_rule & WS_BLANK_AT_EOF)) {
2514
/*
@@ -2522,17 +2525,23 @@ static int match_fragment(struct apply_state *state,
2525
* we are not removing blanks at the end, so we
2526
* should reject the hunk at this position.
2527
*/
2525
- return 0;
2528
+ ret = 0;
2529
+ goto out;
2530
}
2531
2528
- if (match_beginning && current_lno)
2529
- return 0;
2532
+ if (match_beginning && current_lno) {
2533
+ ret = 0;
2534
+ goto out;
2535
+ }
2536
2537
/* Quick hash check */
2532
- for (i = 0; i < preimage_limit; i++)
2538
+ for (i = 0; i < preimage_limit; i++) {
2539
if ((img->line[current_lno + i].flag & LINE_PATCHED) ||
2534
- (preimage->line[i].hash != img->line[current_lno + i].hash))
2535
- return 0;
2540
+ (preimage->line[i].hash != img->line[current_lno + i].hash)) {
2541
+ ret = 0;
2542
+ goto out;
2543
+ }
2544
+ }
2545
2546
if (preimage_limit == preimage->nr) {
2547
/*
@@ -2545,8 +2554,10 @@ static int match_fragment(struct apply_state *state,
2554
if ((match_end
2555
? (current + preimage->len == img->len)
2556
: (current + preimage->len <= img->len)) &&
2548
- !memcmp(img->buf + current, preimage->buf, preimage->len))
2549
- return 1;
2557
+ !memcmp(img->buf + current, preimage->buf, preimage->len)) {
2558
+ ret = 1;
2559
+ goto out;
2560
+ }
2561
} else {
2562
/*
2563
* The preimage extends beyond the end of img, so
@@ -2555,7 +2566,7 @@ static int match_fragment(struct apply_state *state,
2566
* There must be one non-blank context line that match
2567
* a line before the end of img.
2568
*/
2558
- char *buf_end;
2569
+ const char *buf, *buf_end;
2570
2571
buf = preimage->buf;
2572
buf_end = buf;
@@ -2565,8 +2576,10 @@ static int match_fragment(struct apply_state *state,
2576
for ( ; buf < buf_end; buf++)
2577
if (!isspace(*buf))
2578
break;
2568
- if (buf == buf_end)
2569
- return 0;
2579
+ if (buf == buf_end) {
2580
+ ret = 0;
2581
+ goto out;
2582
+ }
2583
}
2584
2585
/*
@@ -2574,12 +2587,16 @@ static int match_fragment(struct apply_state *state,
2587
* fuzzy matching. We collect all the line length information because
2588
* we need it to adjust whitespace if we match.
2589
*/
2577
- if (state->ws_ignore_action == ignore_ws_change)
2578
- return line_by_line_fuzzy_match(img, preimage, postimage,
2579
- current, current_lno, preimage_limit);
2590
+ if (state->ws_ignore_action == ignore_ws_change) {
2591
+ ret = line_by_line_fuzzy_match(img, preimage, postimage,
2592
+ current, current_lno, preimage_limit);
2593
+ goto out;
2594
+ }
2595
2581
- if (state->ws_error_action != correct_ws_error)
2582
- return 0;
2596
+ if (state->ws_error_action != correct_ws_error) {
2597
+ ret = 0;
2598
+ goto out;
2599
+ }
2600
2601
/*
2602
* The hunk does not apply byte-by-byte, but the hash says
@@ -2608,7 +2625,7 @@ static int match_fragment(struct apply_state *state,
2625
* but in this loop we will only handle the part of the
2626
* preimage that falls within the file.
2627
*/
2611
- strbuf_init(&fixed, preimage->len + 1);
2628
+ strbuf_grow(&fixed, preimage->len + 1);
2629
orig = preimage->buf;
2630
target = img->buf + current;
2631
for (i = 0; i < preimage_limit; i++) {
@@ -2644,8 +2661,10 @@ static int match_fragment(struct apply_state *state,
2661
postlen += tgtfix.len;
2662
2663
strbuf_release(&tgtfix);
2647
- if (!match)
2648
- goto unmatch_exit;
2664
+ if (!match) {
2665
+ ret = 0;
2666
+ goto out;
2667
+ }
2668
2669
orig += oldlen;
2670
target += tgtlen;
@@ -2666,9 +2685,13 @@ static int match_fragment(struct apply_state *state,
2685
/* Try fixing the line in the preimage */
2686
ws_fix_copy(&fixed, orig, oldlen, ws_rule, NULL);
2687
2669
- for (j = fixstart; j < fixed.len; j++)
2670
- if (!isspace(fixed.buf[j]))
2671
- goto unmatch_exit;
2688
+ for (j = fixstart; j < fixed.len; j++) {
2689
+ if (!isspace(fixed.buf[j])) {
2690
+ ret = 0;
2691
+ goto out;
2692
+ }
2693
+ }
2694
+
2695
2696
orig += oldlen;
2697
}
@@ -2678,16 +2701,16 @@ static int match_fragment(struct apply_state *state,
2701
* has whitespace breakages unfixed, and fixing them makes the
2702
* hunk match. Update the context lines in the postimage.
2703
*/
2681
- fixed_buf = strbuf_detach(&fixed, &fixed_len);
2704
if (postlen < postimage->len)
2705
postlen = 0;
2706
update_pre_post_images(preimage, postimage,
2685
- fixed_buf, fixed_len, postlen);
2686
- return 1;
2707
+ fixed.buf, fixed.len, postlen);
2708
2688
- unmatch_exit:
2709
+ ret = 1;
2710
+
2711
+out:
2712
strbuf_release(&fixed);
2690
- return 0;
2713
+ return ret;
2714
}
2715
2716
static int find_pos(struct apply_state *state,
t/t3417-rebase-whitespace-fix.sh
+1
@@ -5,6 +5,7 @@ test_description='git rebase --whitespace=fix
5
This test runs git rebase --whitespace=fix and make sure that it works.
6
'
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
11
# prepare initial revision of "file" with a blank line at the end