Skip to content

Commit 2916cfe

Browse files
committed
Merge branch 'pw/diff-color-moved-ws-fix'
Various fixes to "diff --color-moved-ws". * pw/diff-color-moved-ws-fix: diff --color-moved: fix a memory leak diff --color-moved-ws: fix another memory leak diff --color-moved-ws: fix a memory leak diff --color-moved-ws: fix out of bounds string access diff --color-moved-ws: fix double free crash
2 parents 82d0a8c + 47cb16a commit 2916cfe

File tree

1 file changed

+54
-41
lines changed

1 file changed

+54
-41
lines changed

diff.c

Lines changed: 54 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,6 @@ struct moved_entry {
778778
struct hashmap_entry ent;
779779
const struct emitted_diff_symbol *es;
780780
struct moved_entry *next_line;
781-
struct ws_delta *wsd;
782781
};
783782

784783
/**
@@ -795,6 +794,17 @@ struct ws_delta {
795794
};
796795
#define WS_DELTA_INIT { NULL, 0 }
797796

797+
struct moved_block {
798+
struct moved_entry *match;
799+
struct ws_delta wsd;
800+
};
801+
802+
static void moved_block_clear(struct moved_block *b)
803+
{
804+
FREE_AND_NULL(b->wsd.string);
805+
b->match = NULL;
806+
}
807+
798808
static int compute_ws_delta(const struct emitted_diff_symbol *a,
799809
const struct emitted_diff_symbol *b,
800810
struct ws_delta *out)
@@ -803,16 +813,19 @@ static int compute_ws_delta(const struct emitted_diff_symbol *a,
803813
const struct emitted_diff_symbol *shorter = a->len > b->len ? b : a;
804814
int d = longer->len - shorter->len;
805815

816+
if (strncmp(longer->line + d, shorter->line, shorter->len))
817+
return 0;
818+
806819
out->string = xmemdupz(longer->line, d);
807820
out->current_longer = (a == longer);
808821

809-
return !strncmp(longer->line + d, shorter->line, shorter->len);
822+
return 1;
810823
}
811824

812825
static int cmp_in_block_with_wsd(const struct diff_options *o,
813826
const struct moved_entry *cur,
814827
const struct moved_entry *match,
815-
struct moved_entry *pmb,
828+
struct moved_block *pmb,
816829
int n)
817830
{
818831
struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
@@ -832,33 +845,32 @@ static int cmp_in_block_with_wsd(const struct diff_options *o,
832845
if (strcmp(a, b))
833846
return 1;
834847

835-
if (!pmb->wsd)
848+
if (!pmb->wsd.string)
836849
/*
837-
* No white space delta was carried forward? This can happen
838-
* when we exit early in this function and do not carry
839-
* forward ws.
850+
* The white space delta is not active? This can happen
851+
* when we exit early in this function.
840852
*/
841853
return 1;
842854

843855
/*
844-
* The indent changes of the block are known and carried forward in
856+
* The indent changes of the block are known and stored in
845857
* pmb->wsd; however we need to check if the indent changes of the
846858
* current line are still the same as before.
847859
*
848860
* To do so we need to compare 'l' to 'cur', adjusting the
849861
* one of them for the white spaces, depending which was longer.
850862
*/
851863

852-
wslen = strlen(pmb->wsd->string);
853-
if (pmb->wsd->current_longer) {
864+
wslen = strlen(pmb->wsd.string);
865+
if (pmb->wsd.current_longer) {
854866
c += wslen;
855867
cl -= wslen;
856868
} else {
857869
a += wslen;
858870
al -= wslen;
859871
}
860872

861-
if (strcmp(a, c))
873+
if (al != cl || memcmp(a, c, al))
862874
return 1;
863875

864876
return 0;
@@ -900,7 +912,6 @@ static struct moved_entry *prepare_entry(struct diff_options *o,
900912
ret->ent.hash = xdiff_hash_string(l->line, l->len, flags);
901913
ret->es = l;
902914
ret->next_line = NULL;
903-
ret->wsd = NULL;
904915

905916
return ret;
906917
}
@@ -940,76 +951,74 @@ static void add_lines_to_move_detection(struct diff_options *o,
940951
static void pmb_advance_or_null(struct diff_options *o,
941952
struct moved_entry *match,
942953
struct hashmap *hm,
943-
struct moved_entry **pmb,
954+
struct moved_block *pmb,
944955
int pmb_nr)
945956
{
946957
int i;
947958
for (i = 0; i < pmb_nr; i++) {
948-
struct moved_entry *prev = pmb[i];
959+
struct moved_entry *prev = pmb[i].match;
949960
struct moved_entry *cur = (prev && prev->next_line) ?
950961
prev->next_line : NULL;
951962
if (cur && !hm->cmpfn(o, cur, match, NULL)) {
952-
pmb[i] = cur;
963+
pmb[i].match = cur;
953964
} else {
954-
pmb[i] = NULL;
965+
pmb[i].match = NULL;
955966
}
956967
}
957968
}
958969

959970
static void pmb_advance_or_null_multi_match(struct diff_options *o,
960971
struct moved_entry *match,
961972
struct hashmap *hm,
962-
struct moved_entry **pmb,
973+
struct moved_block *pmb,
963974
int pmb_nr, int n)
964975
{
965976
int i;
966977
char *got_match = xcalloc(1, pmb_nr);
967978

968979
for (; match; match = hashmap_get_next(hm, match)) {
969980
for (i = 0; i < pmb_nr; i++) {
970-
struct moved_entry *prev = pmb[i];
981+
struct moved_entry *prev = pmb[i].match;
971982
struct moved_entry *cur = (prev && prev->next_line) ?
972983
prev->next_line : NULL;
973984
if (!cur)
974985
continue;
975-
if (!cmp_in_block_with_wsd(o, cur, match, pmb[i], n))
986+
if (!cmp_in_block_with_wsd(o, cur, match, &pmb[i], n))
976987
got_match[i] |= 1;
977988
}
978989
}
979990

980991
for (i = 0; i < pmb_nr; i++) {
981992
if (got_match[i]) {
982-
/* Carry the white space delta forward */
983-
pmb[i]->next_line->wsd = pmb[i]->wsd;
984-
pmb[i] = pmb[i]->next_line;
993+
/* Advance to the next line */
994+
pmb[i].match = pmb[i].match->next_line;
985995
} else {
986-
if (pmb[i]->wsd) {
987-
free(pmb[i]->wsd->string);
988-
FREE_AND_NULL(pmb[i]->wsd);
989-
}
990-
pmb[i] = NULL;
996+
moved_block_clear(&pmb[i]);
991997
}
992998
}
999+
1000+
free(got_match);
9931001
}
9941002

995-
static int shrink_potential_moved_blocks(struct moved_entry **pmb,
1003+
static int shrink_potential_moved_blocks(struct moved_block *pmb,
9961004
int pmb_nr)
9971005
{
9981006
int lp, rp;
9991007

10001008
/* Shrink the set of potential block to the remaining running */
10011009
for (lp = 0, rp = pmb_nr - 1; lp <= rp;) {
1002-
while (lp < pmb_nr && pmb[lp])
1010+
while (lp < pmb_nr && pmb[lp].match)
10031011
lp++;
10041012
/* lp points at the first NULL now */
10051013

1006-
while (rp > -1 && !pmb[rp])
1014+
while (rp > -1 && !pmb[rp].match)
10071015
rp--;
10081016
/* rp points at the last non-NULL */
10091017

10101018
if (lp < pmb_nr && rp > -1 && lp < rp) {
10111019
pmb[lp] = pmb[rp];
1012-
pmb[rp] = NULL;
1020+
pmb[rp].match = NULL;
1021+
pmb[rp].wsd.string = NULL;
10131022
rp--;
10141023
lp++;
10151024
}
@@ -1056,7 +1065,7 @@ static void mark_color_as_moved(struct diff_options *o,
10561065
struct hashmap *add_lines,
10571066
struct hashmap *del_lines)
10581067
{
1059-
struct moved_entry **pmb = NULL; /* potentially moved blocks */
1068+
struct moved_block *pmb = NULL; /* potentially moved blocks */
10601069
int pmb_nr = 0, pmb_alloc = 0;
10611070
int n, flipped_block = 1, block_length = 0;
10621071

@@ -1085,7 +1094,11 @@ static void mark_color_as_moved(struct diff_options *o,
10851094
}
10861095

10871096
if (!match) {
1097+
int i;
1098+
10881099
adjust_last_block(o, n, block_length);
1100+
for(i = 0; i < pmb_nr; i++)
1101+
moved_block_clear(&pmb[i]);
10891102
pmb_nr = 0;
10901103
block_length = 0;
10911104
continue;
@@ -1113,14 +1126,12 @@ static void mark_color_as_moved(struct diff_options *o,
11131126
ALLOC_GROW(pmb, pmb_nr + 1, pmb_alloc);
11141127
if (o->color_moved_ws_handling &
11151128
COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) {
1116-
struct ws_delta *wsd = xmalloc(sizeof(*match->wsd));
1117-
if (compute_ws_delta(l, match->es, wsd)) {
1118-
match->wsd = wsd;
1119-
pmb[pmb_nr++] = match;
1120-
} else
1121-
free(wsd);
1129+
if (compute_ws_delta(l, match->es,
1130+
&pmb[pmb_nr].wsd))
1131+
pmb[pmb_nr++].match = match;
11221132
} else {
1123-
pmb[pmb_nr++] = match;
1133+
pmb[pmb_nr].wsd.string = NULL;
1134+
pmb[pmb_nr++].match = match;
11241135
}
11251136
}
11261137

@@ -1137,6 +1148,8 @@ static void mark_color_as_moved(struct diff_options *o,
11371148
}
11381149
adjust_last_block(o, n, block_length);
11391150

1151+
for(n = 0; n < pmb_nr; n++)
1152+
moved_block_clear(&pmb[n]);
11401153
free(pmb);
11411154
}
11421155

@@ -5867,8 +5880,8 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
58675880
if (o->color_moved == COLOR_MOVED_ZEBRA_DIM)
58685881
dim_moved_lines(o);
58695882

5870-
hashmap_free(&add_lines, 0);
5871-
hashmap_free(&del_lines, 0);
5883+
hashmap_free(&add_lines, 1);
5884+
hashmap_free(&del_lines, 1);
58725885
}
58735886

58745887
for (i = 0; i < esm.nr; i++)

0 commit comments

Comments
 (0)