Increment/decrement broken in tiny Vim builds from 9.1.0172

[Rob Foehl]

I'd noticed a while ago that increment/decrement (CTRL-A/CTRL-X, of
course) in Fedora-packaged vim-minimal binaries -- essentially tiny
builds -- started producing odd results, e.g. a file containing '1234'
on the first/only line looks like this after CTRL-A anywhere in that
line:

12352

and after another:

123532

and another:

1235332

...and so on. The numbers don't matter much aside from always inserting
earlier digits. Bisecting found the culprit:

╶➤ git bisect good
94b7c3233ef534acc669b3083ed1fe59cf3a090b is the first bad commit
commit 94b7c3233ef534acc669b3083ed1fe59cf3a090b (tag: v9.1.0172)
Author: zeertzjq <zeer...@outlook.com>
Date: Tue Mar 12 21:50:32 2024 +0100

patch 9.1.0172: More code can use ml_get_buf_len() instead of STRLEN()


And reverting the change to ops.c:do_addsub() from that patch does
restore correct behavior:

diff --git a/src/ops.c b/src/ops.c
index 1dd36ab28..0fdbc9c24 100644
--- a/src/ops.c
+++ b/src/ops.c
@@ -3059,12 +3059,14 @@ do_addsub(
// del_char() will also mark line needing displaying
if (todel > 0)
{
- int bytes_after = ml_get_curline_len() - curwin->w_cursor.col;
+ int bytes_after = (int)STRLEN(ml_get_curline())
+ - curwin->w_cursor.col;

// Delete the one character before the insert.
curwin->w_cursor = save_pos;
(void)del_char(FALSE);
- curwin->w_cursor.col = ml_get_curline_len() - bytes_after;
+ curwin->w_cursor.col = (colnr_T)(STRLEN(ml_get_curline())
+ - bytes_after);
--todel;
}
while (todel-- > 0)


It's not obvious why that causes the problem, though -- nor why it only
affects tiny builds. (Fedora-packaged huge builds up through the
current 9.1.0452 package on Fedora 40 don't exhibit this problem.)

I'm out of time for now; figured I'd post what I have, since it might be
more apparent to someone familiar with this code.

-Rob

[Christian Brabandt]

Thanks for the clear analysis. Apparently in tiny mode, we don't need to
get a new allocated pointer to the buffer line and then changing the
line does not invalidate the cached text length. I have now created
https://github.com/vim/vim/pull/15178 as a fix.

Thanks,
Christian
--
Delusions are often functional. A mother's opinions about her children's
beauty, intelligence, goodness, et cetera ad nauseam, keep her from drowning
them at birth.
-- Robert Heinlein

[Rob Foehl]

On Sun, 2024-07-07 at 18:53 +0200, Christian Brabandt wrote:
> Thanks for the clear analysis. Apparently in tiny mode, we don't need to
> get a new allocated pointer to the buffer line and then changing the
> line does not invalidate the cached text length. I have now created
> https://github.com/vim/vim/pull/15178 as a fix.

Thanks, that'd have taken me a while :) That patch seems to work.

-Rob

[Rob Foehl]

On Sun, 2024-07-07 at 13:52 -0400, Rob Foehl wrote:
> That patch seems to work.

As does 9.1.0546.  Thanks!

-Rob