From d89144626e7429d9c499875ed426a6223f9013be Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 20 May 2024 06:15:58 +0800 Subject: vim-patch:9.1.0394: Cannot get a list of positions describing a region Problem: Cannot get a list of positions describing a region (Justin M. Keyes, after v9.1.0120) Solution: Add the getregionpos() function (Shougo Matsushita) fixes: vim/vim#14609 closes: vim/vim#14617 https://github.com/vim/vim/commit/b4757e627e6c83d1c8e5535d4887a82d6a5efdd0 Co-authored-by: Shougo Matsushita Co-authored-by: Justin M. Keyes --- src/nvim/eval.lua | 25 ++++++ src/nvim/eval/funcs.c | 228 +++++++++++++++++++++++++++++++++++--------------- 2 files changed, 184 insertions(+), 69 deletions(-) (limited to 'src') diff --git a/src/nvim/eval.lua b/src/nvim/eval.lua index f0eeca2f10..0653a51f96 100644 --- a/src/nvim/eval.lua +++ b/src/nvim/eval.lua @@ -4414,6 +4414,31 @@ M.funcs = { returns = 'string[]', signature = 'getregion({pos1}, {pos2} [, {opts}])', }, + getregionpos = { + args = { 2, 3 }, + base = 1, + desc = [=[ + Same as |getregion()|, but returns a list of positions + describing the buffer text segments bound by {pos1} and + {pos2}. + The segments are a pair of positions for every line: > + [[{start_pos}, {end_pos}], ...] + < + The position is a |List| with four numbers: + [bufnum, lnum, col, off] + "bufnum" is the buffer number. + "lnum" and "col" are the position in the buffer. The first + column is 1. + The "off" number is zero, unless 'virtualedit' is used. Then + it is the offset in screen columns from the start of the + character. E.g., a position within a or after the last + character. + ]=], + name = 'getregionpos', + params = { { 'pos1', 'table' }, { 'pos2', 'table' }, { 'opts', 'table' } }, + returns = 'integer[][][]', + signature = 'getregionpos({pos1}, {pos2} [, {opts}])', + }, getregtype = { args = { 0, 1 }, base = 1, diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 0b4b79c6ca..2667f4a694 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -2823,24 +2823,25 @@ static char *block_def2str(struct block_def *bd) return ret; } -/// "getregion()" function -static void f_getregion(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) +static int getregionpos(typval_T *argvars, typval_T *rettv, pos_T *p1, pos_T *p2, + bool *const inclusive, MotionType *region_type, oparg_T *oa, + int *const fnum) + FUNC_ATTR_NONNULL_ALL { tv_list_alloc_ret(rettv, kListLenMayKnow); if (tv_check_for_list_arg(argvars, 0) == FAIL || tv_check_for_list_arg(argvars, 1) == FAIL || tv_check_for_opt_dict_arg(argvars, 2) == FAIL) { - return; + return FAIL; } int fnum1 = -1; int fnum2 = -1; - pos_T p1, p2; - if (list2fpos(&argvars[0], &p1, &fnum1, NULL, false) != OK - || list2fpos(&argvars[1], &p2, &fnum2, NULL, false) != OK + if (list2fpos(&argvars[0], p1, &fnum1, NULL, false) != OK + || list2fpos(&argvars[1], p2, &fnum2, NULL, false) != OK || fnum1 != fnum2) { - return; + return FAIL; } bool is_select_exclusive; @@ -2858,108 +2859,123 @@ static void f_getregion(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) type = default_type; } - MotionType region_type = kMTUnknown; if (type[0] == 'v' && type[1] == NUL) { - region_type = kMTCharWise; + *region_type = kMTCharWise; } else if (type[0] == 'V' && type[1] == NUL) { - region_type = kMTLineWise; + *region_type = kMTLineWise; } else if (type[0] == Ctrl_V && type[1] == NUL) { - region_type = kMTBlockWise; + *region_type = kMTBlockWise; } else { semsg(_(e_invargNval), "type", type); - return; + return FAIL; } buf_T *findbuf = fnum1 != 0 ? buflist_findnr(fnum1) : curbuf; + *fnum = fnum1 != 0 ? fnum1 : curbuf->b_fnum; if (findbuf == NULL || findbuf->b_ml.ml_mfp == NULL) { emsg(_(e_buffer_is_not_loaded)); - return; + return FAIL; } - if (p1.lnum < 1 || p1.lnum > findbuf->b_ml.ml_line_count) { - semsg(_(e_invalid_line_number_nr), p1.lnum); - return; + if (p1->lnum < 1 || p1->lnum > findbuf->b_ml.ml_line_count) { + semsg(_(e_invalid_line_number_nr), p1->lnum); + return FAIL; } - if (p1.col == MAXCOL) { - p1.col = ml_get_buf_len(findbuf, p1.lnum) + 1; - } else if (p1.col < 1 || p1.col > ml_get_buf_len(findbuf, p1.lnum) + 1) { - semsg(_(e_invalid_column_number_nr), p1.col); - return; + if (p1->col == MAXCOL) { + p1->col = ml_get_buf_len(findbuf, p1->lnum) + 1; + } else if (p1->col < 1 || p1->col > ml_get_buf_len(findbuf, p1->lnum) + 1) { + semsg(_(e_invalid_column_number_nr), p1->col); + return FAIL; } - if (p2.lnum < 1 || p2.lnum > findbuf->b_ml.ml_line_count) { - semsg(_(e_invalid_line_number_nr), p2.lnum); - return; + if (p2->lnum < 1 || p2->lnum > findbuf->b_ml.ml_line_count) { + semsg(_(e_invalid_line_number_nr), p2->lnum); + return FAIL; } - if (p2.col == MAXCOL) { - p2.col = ml_get_buf_len(findbuf, p2.lnum) + 1; - } else if (p2.col < 1 || p2.col > ml_get_buf_len(findbuf, p2.lnum) + 1) { - semsg(_(e_invalid_column_number_nr), p2.col); - return; + if (p2->col == MAXCOL) { + p2->col = ml_get_buf_len(findbuf, p2->lnum) + 1; + } else if (p2->col < 1 || p2->col > ml_get_buf_len(findbuf, p2->lnum) + 1) { + semsg(_(e_invalid_column_number_nr), p2->col); + return FAIL; } - buf_T *const save_curbuf = curbuf; curbuf = findbuf; curwin->w_buffer = curbuf; - const TriState save_virtual = virtual_op; virtual_op = virtual_active(curwin); - // NOTE: Adjust is needed. - p1.col--; - p2.col--; + // NOTE: Adjustment is needed. + p1->col--; + p2->col--; - if (!lt(p1, p2)) { + if (!lt(*p1, *p2)) { // swap position - pos_T p = p1; - p1 = p2; - p2 = p; + pos_T p = *p1; + *p1 = *p2; + *p2 = p; } - oparg_T oa; - bool inclusive = true; - - if (region_type == kMTCharWise) { + if (*region_type == kMTCharWise) { // handle 'selection' == "exclusive" - if (is_select_exclusive && !equalpos(p1, p2)) { - if (p2.coladd > 0) { - p2.coladd--; - } else if (p2.col > 0) { - p2.col--; - mark_mb_adjustpos(curbuf, &p2); - } else if (p2.lnum > 1) { - p2.lnum--; - p2.col = ml_get_len(p2.lnum); - if (p2.col > 0) { - p2.col--; - mark_mb_adjustpos(curbuf, &p2); + if (is_select_exclusive && !equalpos(*p1, *p2)) { + if (p2->coladd > 0) { + p2->coladd--; + } else if (p2->col > 0) { + p2->col--; + mark_mb_adjustpos(curbuf, p2); + } else if (p2->lnum > 1) { + p2->lnum--; + p2->col = ml_get_len(p2->lnum); + if (p2->col > 0) { + p2->col--; + mark_mb_adjustpos(curbuf, p2); } } } // if fp2 is on NUL (empty line) inclusive becomes false - if (*ml_get_pos(&p2) == NUL && !virtual_op) { - inclusive = false; + if (*ml_get_pos(p2) == NUL && !virtual_op) { + *inclusive = false; } - } else if (region_type == kMTBlockWise) { + } else if (*region_type == kMTBlockWise) { colnr_T sc1, ec1, sc2, ec2; - getvvcol(curwin, &p1, &sc1, NULL, &ec1); - getvvcol(curwin, &p2, &sc2, NULL, &ec2); - oa.motion_type = kMTBlockWise; - oa.inclusive = true; - oa.op_type = OP_NOP; - oa.start = p1; - oa.end = p2; - oa.start_vcol = MIN(sc1, sc2); + getvvcol(curwin, p1, &sc1, NULL, &ec1); + getvvcol(curwin, p2, &sc2, NULL, &ec2); + oa->motion_type = kMTBlockWise; + oa->inclusive = true; + oa->op_type = OP_NOP; + oa->start = *p1; + oa->end = *p2; + oa->start_vcol = MIN(sc1, sc2); if (is_select_exclusive && ec1 < sc2 && 0 < sc2 && ec2 > ec1) { - oa.end_vcol = sc2 - 1; + oa->end_vcol = sc2 - 1; } else { - oa.end_vcol = MAX(ec1, ec2); + oa->end_vcol = MAX(ec1, ec2); } } // Include the trailing byte of a multi-byte char. - int l = utfc_ptr2len(ml_get_pos(&p2)); + int l = utfc_ptr2len(ml_get_pos(p2)); if (l > 1) { - p2.col += l - 1; + p2->col += l - 1; + } + + return OK; +} + +/// "getregion()" function +static void f_getregion(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) +{ + buf_T *const save_curbuf = curbuf; + const TriState save_virtual = virtual_op; + + pos_T p1, p2; + bool inclusive = true; + MotionType region_type = kMTUnknown; + oparg_T oa; + int fnum; + + if (getregionpos(argvars, rettv, + &p1, &p2, &inclusive, ®ion_type, &oa, &fnum) == FAIL) { + return; } for (linenr_T lnum = p1.lnum; lnum <= p2.lnum; lnum++) { @@ -2983,6 +2999,80 @@ static void f_getregion(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) tv_list_append_allocated_string(rettv->vval.v_list, akt); } + // getregionpos() breaks curbuf and virtual_op + curbuf = save_curbuf; + curwin->w_buffer = curbuf; + virtual_op = save_virtual; +} + +static void add_regionpos_range(typval_T *rettv, int bufnr, int lnum1, int col1, int coladd1, + int lnum2, int col2, int coladd2) +{ + list_T *l1 = tv_list_alloc(2); + tv_list_append_list(rettv->vval.v_list, l1); + + list_T *l2 = tv_list_alloc(4); + tv_list_append_list(l1, l2); + + list_T *l3 = tv_list_alloc(4); + tv_list_append_list(l1, l3); + + buf_T *findbuf = bufnr != 0 ? buflist_findnr(bufnr) : curbuf; + + int max_col1 = ml_get_buf_len(findbuf, lnum1); + tv_list_append_number(l2, bufnr); + tv_list_append_number(l2, lnum1); + tv_list_append_number(l2, col1 > max_col1 ? max_col1 : col1); + tv_list_append_number(l2, coladd1); + + int max_col2 = ml_get_buf_len(findbuf, lnum2); + tv_list_append_number(l3, bufnr); + tv_list_append_number(l3, lnum2); + tv_list_append_number(l3, col2 > max_col2 ? max_col2 : col2); + tv_list_append_number(l3, coladd2); +} + +/// "getregionpos()" function +static void f_getregionpos(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) +{ + buf_T *const save_curbuf = curbuf; + const TriState save_virtual = virtual_op; + + pos_T p1, p2; + bool inclusive = true; + MotionType region_type = kMTUnknown; + oparg_T oa; + int fnum; + + if (getregionpos(argvars, rettv, + &p1, &p2, &inclusive, ®ion_type, &oa, &fnum) == FAIL) { + return; + } + + for (linenr_T lnum = p1.lnum; lnum <= p2.lnum; lnum++) { + int start_col, end_col; + + if (region_type == kMTLineWise) { + start_col = 1; + end_col = MAXCOL; + } else if (region_type == kMTBlockWise) { + struct block_def bd; + block_prep(&oa, &bd, lnum, false); + start_col = bd.start_vcol + 1; + end_col = bd.end_vcol; + } else if (p1.lnum < lnum && lnum < p2.lnum) { + start_col = 1; + end_col = MAXCOL; + } else { + start_col = p1.lnum == lnum ? p1.col + 1 : 1; + end_col = p2.lnum == lnum ? p2.col + 1 : MAXCOL; + } + + add_regionpos_range(rettv, fnum, lnum, start_col, + p1.coladd, lnum, end_col, p2.coladd); + } + + // getregionpos() may change curbuf and virtual_op curbuf = save_curbuf; curwin->w_buffer = curbuf; virtual_op = save_virtual; -- cgit From 3383603c134944d374eb0814a2f707a7e3e89b43 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 20 May 2024 06:44:19 +0800 Subject: vim-patch:9.1.0395: getregionpos() may leak memory on error Problem: regionpos may leak memory on error, coverity complains about dereferencing Null pointer Solution: free all list pointers (after v9.1.394), return early if buflist_findnr() returns NULL closes: vim/vim#14731 https://github.com/vim/vim/commit/b8ecedce79149ac6b994177e9a68979f86065cb1 Co-authored-by: Christian Brabandt --- src/nvim/eval/funcs.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 2667f4a694..a5d6124eb3 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -3008,6 +3008,11 @@ static void f_getregion(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) static void add_regionpos_range(typval_T *rettv, int bufnr, int lnum1, int col1, int coladd1, int lnum2, int col2, int coladd2) { + buf_T *findbuf = bufnr != 0 ? buflist_findnr(bufnr) : curbuf; + if (findbuf == NULL || findbuf->b_ml.ml_mfp == NULL) { + return; + } + list_T *l1 = tv_list_alloc(2); tv_list_append_list(rettv->vval.v_list, l1); @@ -3017,8 +3022,6 @@ static void add_regionpos_range(typval_T *rettv, int bufnr, int lnum1, int col1, list_T *l3 = tv_list_alloc(4); tv_list_append_list(l1, l3); - buf_T *findbuf = bufnr != 0 ? buflist_findnr(bufnr) : curbuf; - int max_col1 = ml_get_buf_len(findbuf, lnum1); tv_list_append_number(l2, bufnr); tv_list_append_number(l2, lnum1); -- cgit From e0259b9466a0dd62b74d4aa195b3c5e6c7a183d0 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 20 May 2024 20:50:32 +0800 Subject: vim-patch:9.1.0423: getregionpos() wrong with blockwise mode and multibyte Problem: getregionpos() wrong with blockwise mode and multibyte. Solution: Use textcol and textlen instead of start_vcol and end_vcol. Handle coladd properly (zeertzjq). Also remove unnecessary buflist_findnr() in add_regionpos_range(), as getregionpos() has already switched buffer. closes: vim/vim#14805 https://github.com/vim/vim/commit/c95e64f41f7f6d1bdc95b047ae9b369743c8637b --- src/nvim/eval.lua | 10 +++--- src/nvim/eval/funcs.c | 85 ++++++++++++++++++++++++++------------------------- src/nvim/ops.c | 5 ++- 3 files changed, 53 insertions(+), 47 deletions(-) (limited to 'src') diff --git a/src/nvim/eval.lua b/src/nvim/eval.lua index 0653a51f96..58f2ac0acf 100644 --- a/src/nvim/eval.lua +++ b/src/nvim/eval.lua @@ -4429,10 +4429,12 @@ M.funcs = { "bufnum" is the buffer number. "lnum" and "col" are the position in the buffer. The first column is 1. - The "off" number is zero, unless 'virtualedit' is used. Then - it is the offset in screen columns from the start of the - character. E.g., a position within a or after the last - character. + If the "off" number of a starting position is non-zero, it is + the offset in screen columns from the start of the character. + E.g., a position within a or after the last character. + If the "off" number of an ending position is non-zero, it is + the character's number of cells included in the selection, + otherwise the whole character is included. ]=], name = 'getregionpos', params = { { 'pos1', 'table' }, { 'pos2', 'table' }, { 'opts', 'table' } }, diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index a5d6124eb3..0240e08dad 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -2824,8 +2824,7 @@ static char *block_def2str(struct block_def *bd) } static int getregionpos(typval_T *argvars, typval_T *rettv, pos_T *p1, pos_T *p2, - bool *const inclusive, MotionType *region_type, oparg_T *oa, - int *const fnum) + bool *const inclusive, MotionType *region_type, oparg_T *oa) FUNC_ATTR_NONNULL_ALL { tv_list_alloc_ret(rettv, kListLenMayKnow); @@ -2871,7 +2870,6 @@ static int getregionpos(typval_T *argvars, typval_T *rettv, pos_T *p1, pos_T *p2 } buf_T *findbuf = fnum1 != 0 ? buflist_findnr(fnum1) : curbuf; - *fnum = fnum1 != 0 ? fnum1 : curbuf->b_fnum; if (findbuf == NULL || findbuf->b_ml.ml_mfp == NULL) { emsg(_(e_buffer_is_not_loaded)); return FAIL; @@ -2971,10 +2969,8 @@ static void f_getregion(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) bool inclusive = true; MotionType region_type = kMTUnknown; oparg_T oa; - int fnum; - if (getregionpos(argvars, rettv, - &p1, &p2, &inclusive, ®ion_type, &oa, &fnum) == FAIL) { + if (getregionpos(argvars, rettv, &p1, &p2, &inclusive, ®ion_type, &oa) == FAIL) { return; } @@ -2999,20 +2995,14 @@ static void f_getregion(typval_T *argvars, typval_T *rettv, EvalFuncData fptr) tv_list_append_allocated_string(rettv->vval.v_list, akt); } - // getregionpos() breaks curbuf and virtual_op + // getregionpos() may change curbuf and virtual_op curbuf = save_curbuf; curwin->w_buffer = curbuf; virtual_op = save_virtual; } -static void add_regionpos_range(typval_T *rettv, int bufnr, int lnum1, int col1, int coladd1, - int lnum2, int col2, int coladd2) +static void add_regionpos_range(typval_T *rettv, pos_T p1, pos_T p2) { - buf_T *findbuf = bufnr != 0 ? buflist_findnr(bufnr) : curbuf; - if (findbuf == NULL || findbuf->b_ml.ml_mfp == NULL) { - return; - } - list_T *l1 = tv_list_alloc(2); tv_list_append_list(rettv->vval.v_list, l1); @@ -3022,17 +3012,17 @@ static void add_regionpos_range(typval_T *rettv, int bufnr, int lnum1, int col1, list_T *l3 = tv_list_alloc(4); tv_list_append_list(l1, l3); - int max_col1 = ml_get_buf_len(findbuf, lnum1); - tv_list_append_number(l2, bufnr); - tv_list_append_number(l2, lnum1); - tv_list_append_number(l2, col1 > max_col1 ? max_col1 : col1); - tv_list_append_number(l2, coladd1); + int max_col1 = ml_get_len(p1.lnum); + tv_list_append_number(l2, curbuf->b_fnum); + tv_list_append_number(l2, p1.lnum); + tv_list_append_number(l2, p1.col > max_col1 ? max_col1 : p1.col); + tv_list_append_number(l2, p1.coladd); - int max_col2 = ml_get_buf_len(findbuf, lnum2); - tv_list_append_number(l3, bufnr); - tv_list_append_number(l3, lnum2); - tv_list_append_number(l3, col2 > max_col2 ? max_col2 : col2); - tv_list_append_number(l3, coladd2); + int max_col2 = ml_get_len(p2.lnum); + tv_list_append_number(l3, curbuf->b_fnum); + tv_list_append_number(l3, p2.lnum); + tv_list_append_number(l3, p2.col > max_col2 ? max_col2 : p2.col); + tv_list_append_number(l3, p2.coladd); } /// "getregionpos()" function @@ -3045,34 +3035,45 @@ static void f_getregionpos(typval_T *argvars, typval_T *rettv, EvalFuncData fptr bool inclusive = true; MotionType region_type = kMTUnknown; oparg_T oa; - int fnum; - if (getregionpos(argvars, rettv, - &p1, &p2, &inclusive, ®ion_type, &oa, &fnum) == FAIL) { + if (getregionpos(argvars, rettv, &p1, &p2, &inclusive, ®ion_type, &oa) == FAIL) { return; } for (linenr_T lnum = p1.lnum; lnum <= p2.lnum; lnum++) { - int start_col, end_col; + struct block_def bd; + pos_T ret_p1, ret_p2; if (region_type == kMTLineWise) { - start_col = 1; - end_col = MAXCOL; - } else if (region_type == kMTBlockWise) { - struct block_def bd; - block_prep(&oa, &bd, lnum, false); - start_col = bd.start_vcol + 1; - end_col = bd.end_vcol; - } else if (p1.lnum < lnum && lnum < p2.lnum) { - start_col = 1; - end_col = MAXCOL; + ret_p1.col = 1; + ret_p1.coladd = 0; + ret_p2.col = MAXCOL; + ret_p2.coladd = 0; } else { - start_col = p1.lnum == lnum ? p1.col + 1 : 1; - end_col = p2.lnum == lnum ? p2.col + 1 : MAXCOL; + if (region_type == kMTBlockWise) { + block_prep(&oa, &bd, lnum, false); + } else { + charwise_block_prep(p1, p2, &bd, lnum, inclusive); + } + if (bd.startspaces > 0) { + ret_p1.col = bd.textcol; + ret_p1.coladd = bd.start_char_vcols - bd.startspaces; + } else { + ret_p1.col = bd.textcol + 1; + ret_p1.coladd = 0; + } + if (bd.endspaces > 0) { + ret_p2.col = bd.textcol + bd.textlen + 1; + ret_p2.coladd = bd.endspaces; + } else { + ret_p2.col = bd.textcol + bd.textlen; + ret_p2.coladd = 0; + } } - add_regionpos_range(rettv, fnum, lnum, start_col, - p1.coladd, lnum, end_col, p2.coladd); + ret_p1.lnum = lnum; + ret_p2.lnum = lnum; + add_regionpos_range(rettv, ret_p1, ret_p2); } // getregionpos() may change curbuf and virtual_op diff --git a/src/nvim/ops.c b/src/nvim/ops.c index d54eea0f3e..707cf5ca9a 100644 --- a/src/nvim/ops.c +++ b/src/nvim/ops.c @@ -4258,6 +4258,7 @@ void charwise_block_prep(pos_T start, pos_T end, struct block_def *bdp, linenr_T char *p = ml_get(lnum); bdp->startspaces = 0; bdp->endspaces = 0; + bdp->start_char_vcols = 0; if (lnum == start.lnum) { startcol = start.col; @@ -4265,7 +4266,8 @@ void charwise_block_prep(pos_T start, pos_T end, struct block_def *bdp, linenr_T getvcol(curwin, &start, &cs, NULL, &ce); if (ce != cs && start.coladd > 0) { // Part of a tab selected -- but don't double-count it. - bdp->startspaces = (ce - cs + 1) - start.coladd; + bdp->start_char_vcols = ce - cs + 1; + bdp->startspaces = bdp->start_char_vcols - start.coladd; if (bdp->startspaces < 0) { bdp->startspaces = 0; } @@ -4303,6 +4305,7 @@ void charwise_block_prep(pos_T start, pos_T end, struct block_def *bdp, linenr_T } else { bdp->textlen = endcol - startcol + inclusive; } + bdp->textcol = startcol; bdp->textstart = p + startcol; } -- cgit