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 -0000 1.22
+++ vi/v_txt.c 12 May 2013 17:24:25 -0000
@@ -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;
}
}
