On Sat, May 11, 2013 at 05:33:20PM -0600, Todd C. Miller wrote:
Good catch, I know folks who have hit this bug but I was never able
to reproduce it. Moving the isblank() check should be safe since
trailing blanks are trimmed earlier on so we won't exit the loop
prematurely. I see you didn't change the TXT_ALTWERASE case, though.
It looks like that also needs a fix but the break there will break
out of the switch statement, not a loop. However, since trailing
blanks have already been trimmed I think that check is effectively
a no-op and could simply be removed.
Thanks for the review. Here's an updated diff. If anyone's curious this
bug is about one month short of being 20 years old.
Index: vi/v_txt.c
===
RCS file: /cvs/src/usr.bin/vi/vi/v_txt.c,v
retrieving revision 1.22
diff -u -p -r1.22 v_txt.c
--- vi/v_txt.c 27 Oct 2009 23:59:48 - 1.22
+++ vi/v_txt.c 12 May 2013 17:24:25 -
@@ -1120,12 +1120,12 @@ leftmargin: tp-lb[tp-cno - 1] = ' ';
*/
if (LF_ISSET(TXT_TTYWERASE))
while (tp-cno max) {
+ if (isblank(tp-lb[tp-cno - 1]))
+ break;
--tp-cno;
++tp-owrite;
if (FL_ISSET(is_flags, IS_RUNNING))
tp-lb[tp-cno] = ' ';
- if (isblank(tp-lb[tp-cno - 1]))
- break;
}
else {
if (LF_ISSET(TXT_ALTWERASE)) {
@@ -1133,19 +1133,17 @@ leftmargin: tp-lb[tp-cno - 1] = ' ';
++tp-owrite;
if (FL_ISSET(is_flags, IS_RUNNING))
tp-lb[tp-cno] = ' ';
- if (isblank(tp-lb[tp-cno - 1]))
- break;
}
if (tp-cno max)
tmp = inword(tp-lb[tp-cno - 1]);
while (tp-cno max) {
+ if (tmp != inword(tp-lb[tp-cno - 1])
+ || isblank(tp-lb[tp-cno - 1]))
+ break;
--tp-cno;
++tp-owrite;
if (FL_ISSET(is_flags, IS_RUNNING))
tp-lb[tp-cno] = ' ';
- if (tmp != inword(tp-lb[tp-cno - 1])
- || isblank(tp-lb[tp-cno - 1]))
- break;
}
}