Skip to content

Commit 17a6d69

Browse files
glepnirchrisbra
authored andcommitted
patch 9.1.1628: fuzzy.c has a few issues
Problem: fuzzy.c has a few issues Solution: Use Vims memory management, update style (glepnir) Problem: - Missing cleanup of lmatchpos lists causing memory leaks - Missing error handling for list operations - Use of malloc() instead of Vim's alloc() functions - Inconsistent C-style comments - Missing null pointer checks for memory allocation - Incorrect use of vim_free() for list objects Solution: - Add proper cleanup of lmatchpos in done section using list_free() - Set lmatchpos to NULL after successful transfer to avoid confusion - Add error handling for list_append_tv() failures - Replace malloc() with alloc() and add null pointer checks - Convert C-style comments to C++ style for consistency - Fix vim_free() calls to use list_free() for list objects closes: #17984 Signed-off-by: glepnir <[email protected]> Signed-off-by: Christian Brabandt <[email protected]>
1 parent c93a2b3 commit 17a6d69

2 files changed

Lines changed: 82 additions & 69 deletions

File tree

src/fuzzy.c

Lines changed: 80 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ static score_t match_positions(char_u *needle, char_u *haystack, int_u *position
4949
static int has_match(char_u *needle, char_u *haystack);
5050

5151
#define SCORE_MAX INFINITY
52-
#define SCORE_MIN -INFINITY
52+
#define SCORE_MIN (-INFINITY)
5353
#define SCORE_SCALE 1000
5454

5555
typedef struct
@@ -72,12 +72,12 @@ typedef struct
7272
*/
7373
int
7474
fuzzy_match(
75-
char_u *str,
76-
char_u *pat_arg,
77-
int matchseq,
78-
int *outScore,
79-
int_u *matches,
80-
int maxMatches)
75+
char_u *str,
76+
char_u *pat_arg,
77+
int matchseq,
78+
int *outScore,
79+
int_u *matches,
80+
int maxMatches)
8181
{
8282
char_u *save_pat;
8383
char_u *pat;
@@ -206,21 +206,20 @@ fuzzy_match_item_compare(const void *s1, const void *s2)
206206
*/
207207
static void
208208
fuzzy_match_in_list(
209-
list_T *l,
210-
char_u *str,
211-
int matchseq,
212-
char_u *key,
213-
callback_T *item_cb,
214-
int retmatchpos,
215-
list_T *fmatchlist,
216-
long max_matches)
209+
list_T *l,
210+
char_u *str,
211+
int matchseq,
212+
char_u *key,
213+
callback_T *item_cb,
214+
int retmatchpos,
215+
list_T *fmatchlist,
216+
long max_matches)
217217
{
218-
long len;
219-
fuzzyItem_T *items;
220-
listitem_T *li;
221-
int i = 0;
222-
long match_count = 0;
223-
int_u matches[FUZZY_MATCH_MAX_LEN];
218+
long len;
219+
fuzzyItem_T *items;
220+
listitem_T *li;
221+
long match_count = 0;
222+
int_u matches[FUZZY_MATCH_MAX_LEN];
224223

225224
len = list_len(l);
226225
if (len == 0)
@@ -285,13 +284,13 @@ fuzzy_match_in_list(
285284
items[match_count].score = score;
286285
items[match_count].pat = str;
287286
items[match_count].startpos = matches[0];
288-
if (itemstr_allocate)
289-
items[match_count].itemstr = vim_strsave(itemstr);
290-
else
291-
items[match_count].itemstr = itemstr;
287+
items[match_count].itemstr = itemstr_allocate
288+
? vim_strsave(itemstr) : itemstr;
292289
items[match_count].itemstr_allocated = itemstr_allocate;
293290

294-
// Copy the list of matching positions in itemstr to a list
291+
// Copy the list of matching positions in itemstr to a list, if
292+
// "retmatchpos" is set.
293+
if (retmatchpos)
295294
{
296295
int j = 0;
297296
char_u *p;
@@ -347,8 +346,11 @@ fuzzy_match_in_list(
347346
retlist = fmatchlist;
348347

349348
// Copy the matching strings to the return list
350-
for (i = 0; i < match_count; i++)
351-
list_append_tv(retlist, &items[i].item->li_tv);
349+
for (int i = 0; i < match_count; i++)
350+
{
351+
if (list_append_tv(retlist, &items[i].item->li_tv) == FAIL)
352+
goto done;
353+
}
352354

353355
// next copy the list of matching positions
354356
if (retmatchpos)
@@ -358,26 +360,40 @@ fuzzy_match_in_list(
358360
goto done;
359361
retlist = li->li_tv.vval.v_list;
360362

361-
for (i = 0; i < match_count; i++)
362-
if (items[i].lmatchpos != NULL
363-
&& list_append_list(retlist, items[i].lmatchpos) == FAIL)
364-
goto done;
363+
for (int i = 0; i < match_count; i++)
364+
{
365+
if (items[i].lmatchpos != NULL)
366+
{
367+
if (list_append_list(retlist, items[i].lmatchpos) == OK)
368+
items[i].lmatchpos = NULL;
369+
else
370+
goto done;
371+
372+
}
373+
}
365374

366375
// copy the matching scores
367376
li = list_find(fmatchlist, -1);
368377
if (li == NULL || li->li_tv.vval.v_list == NULL)
369378
goto done;
370379
retlist = li->li_tv.vval.v_list;
371-
for (i = 0; i < match_count; i++)
380+
for (int i = 0; i < match_count; i++)
381+
{
372382
if (list_append_number(retlist, items[i].score) == FAIL)
373383
goto done;
384+
}
374385
}
375386
}
376387

377388
done:
378-
for (i = 0; i < match_count; i++)
389+
for (int i = 0; i < match_count; i++)
390+
{
379391
if (items[i].itemstr_allocated)
380392
vim_free(items[i].itemstr);
393+
394+
if (items[i].lmatchpos)
395+
list_free(items[i].lmatchpos);
396+
}
381397
vim_free(items);
382398
}
383399

@@ -480,23 +496,23 @@ do_fuzzymatch(typval_T *argvars, typval_T *rettv, int retmatchpos)
480496
goto done;
481497
if (list_append_list(rettv->vval.v_list, l) == FAIL)
482498
{
483-
vim_free(l);
499+
list_free(l);
484500
goto done;
485501
}
486502
l = list_alloc();
487503
if (l == NULL)
488504
goto done;
489505
if (list_append_list(rettv->vval.v_list, l) == FAIL)
490506
{
491-
vim_free(l);
507+
list_free(l);
492508
goto done;
493509
}
494510
l = list_alloc();
495511
if (l == NULL)
496512
goto done;
497513
if (list_append_list(rettv->vval.v_list, l) == FAIL)
498514
{
499-
vim_free(l);
515+
list_free(l);
500516
goto done;
501517
}
502518
}
@@ -1020,7 +1036,7 @@ match_row(const match_struct *match, int row, score_t *curr_D,
10201036
score_t prev_score = SCORE_MIN;
10211037
score_t gap_score = i == n - 1 ? SCORE_GAP_TRAILING : SCORE_GAP_INNER;
10221038

1023-
/* These will not be used with this value, but not all compilers see it */
1039+
// These will not be used with this value, but not all compilers see it
10241040
score_t prev_M = SCORE_MIN, prev_D = SCORE_MIN;
10251041

10261042
for (int j = 0; j < m; j++)
@@ -1036,7 +1052,7 @@ match_row(const match_struct *match, int row, score_t *curr_D,
10361052
{ /* i > 0 && j > 0*/
10371053
score = MAX(
10381054
prev_M + match_bonus[j],
1039-
/* consecutive match, doesn't stack with match_bonus */
1055+
// consecutive match, doesn't stack with match_bonus
10401056
prev_D + SCORE_MATCH_CONSECUTIVE);
10411057
}
10421058
prev_D = last_D[j];
@@ -1068,62 +1084,57 @@ match_positions(char_u *needle, char_u *haystack, int_u *positions)
10681084

10691085
if (m > MATCH_MAX_LEN || n > m)
10701086
{
1071-
/*
1072-
* Unreasonably large candidate: return no score
1073-
* If it is a valid match it will still be returned, it will
1074-
* just be ranked below any reasonably sized candidates
1075-
*/
1087+
// Unreasonably large candidate: return no score
1088+
// If it is a valid match it will still be returned, it will
1089+
// just be ranked below any reasonably sized candidates
10761090
return SCORE_MIN;
10771091
}
10781092
else if (n == m)
10791093
{
1080-
/* Since this method can only be called with a haystack which
1081-
* matches needle. If the lengths of the strings are equal the
1082-
* strings themselves must also be equal (ignoring case).
1083-
*/
1094+
// Since this method can only be called with a haystack which
1095+
// matches needle. If the lengths of the strings are equal the
1096+
// strings themselves must also be equal (ignoring case).
10841097
if (positions)
10851098
for (int i = 0; i < n; i++)
10861099
positions[i] = i;
10871100
return SCORE_MAX;
10881101
}
10891102

1090-
/*
1091-
* D[][] Stores the best score for this position ending with a match.
1092-
* M[][] Stores the best possible score at this position.
1093-
*/
1103+
// D[][] Stores the best score for this position ending with a match.
1104+
// M[][] Stores the best possible score at this position.
10941105
score_t (*D)[MATCH_MAX_LEN], (*M)[MATCH_MAX_LEN];
1095-
M = malloc(sizeof(score_t) * MATCH_MAX_LEN * n);
1096-
D = malloc(sizeof(score_t) * MATCH_MAX_LEN * n);
1106+
M = alloc(sizeof(score_t) * MATCH_MAX_LEN * n);
1107+
if (!M)
1108+
return SCORE_MIN;
1109+
D = alloc(sizeof(score_t) * MATCH_MAX_LEN * n);
1110+
if (!D)
1111+
return SCORE_MIN;
10971112

10981113
match_row(&match, 0, D[0], M[0], D[0], M[0]);
10991114
for (int i = 1; i < n; i++)
11001115
match_row(&match, i, D[i], M[i], D[i - 1], M[i - 1]);
11011116

1102-
/* backtrace to find the positions of optimal matching */
1117+
// backtrace to find the positions of optimal matching
11031118
if (positions)
11041119
{
11051120
int match_required = 0;
11061121
for (int i = n - 1, j = m - 1; i >= 0; i--)
11071122
{
11081123
for (; j >= 0; j--)
11091124
{
1110-
/*
1111-
* There may be multiple paths which result in
1112-
* the optimal weight.
1113-
*
1114-
* For simplicity, we will pick the first one
1115-
* we encounter, the latest in the candidate
1116-
* string.
1117-
*/
1125+
// There may be multiple paths which result in
1126+
// the optimal weight.
1127+
//
1128+
// For simplicity, we will pick the first one
1129+
// we encounter, the latest in the candidate
1130+
// string.
11181131
if (D[i][j] != SCORE_MIN &&
11191132
(match_required || D[i][j] == M[i][j]))
11201133
{
1121-
/* If this score was determined using
1122-
* SCORE_MATCH_CONSECUTIVE, the
1123-
* previous character MUST be a match
1124-
*/
1125-
match_required =
1126-
i && j &&
1134+
// If this score was determined using
1135+
// SCORE_MATCH_CONSECUTIVE, the
1136+
// previous character MUST be a match
1137+
match_required = i && j &&
11271138
M[i][j] == D[i - 1][j - 1] + SCORE_MATCH_CONSECUTIVE;
11281139
positions[i] = j--;
11291140
break;

src/version.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,8 @@ static char *(features[]) =
719719

720720
static int included_patches[] =
721721
{ /* Add new patch number below this line */
722+
/**/
723+
1628,
722724
/**/
723725
1627,
724726
/**/

0 commit comments

Comments
 (0)