> Delivered-To: marty...@venck.us
> Received: by 10.100.125.2 with SMTP id x2cs7548anc;
> Wed, 8 Dec 2010 00:00:53 -0800 (PST)
> Received: by 10.231.32.197 with SMTP id e5mr738073ibd.188.1291795252925;
> Wed, 08 Dec 2010 00:00:52 -0800 (PST)
> Return-Path: <owner-tech+m22...@openbsd.org>
> Received: from shear.ucar.edu (lists.openbsd.org [192.43.244.163])
> by mx.google.com with ESMTP id
> gy42si1064435ibb.88.2010.12.08.00.00.52;
> Wed, 08 Dec 2010 00:00:52 -0800 (PST)
> Received-SPF: pass (google.com: manual fallback record for domain of
> owner-tech+m22...@openbsd.org designates 192.43.244.163 as permitted sender)
> client-ip=192.43.244.163;
> Authentication-Results: mx.google.com; spf=pass (google.com: manual fallback
> record for domain of owner-tech+m22...@openbsd.org designates 192.43.244.163
> as permitted sender) smtp.mail=owner-tech+m22...@openbsd.org
> Received: from openbsd.org (localhost.ucar.edu [127.0.0.1])
> by shear.ucar.edu (8.14.3/8.14.3) with ESMTP id oB87xxwI017467;
> Wed, 8 Dec 2010 00:59:59 -0700 (MST)
> Received: from mail.boxsoft.com (dsl092-062-211.lax1.dsl.speakeasy.net
> [66.92.62.211])
> by shear.ucar.edu (8.14.3/8.14.3) with ESMTP id oB87we7E023081
> (version=TLSv1/SSLv3 cipher=DHE-DSS-AES256-SHA bits=256 verify=NO)
> for <tech@openbsd.org>; Wed, 8 Dec 2010 00:58:42 -0700 (MST)
> Received: from mail.boxsoft.com (sidster@localhost [127.0.0.1])
> by mail.boxsoft.com (8.14.3/8.14.3) with ESMTP id oB87weGu028472
> for <tech@openbsd.org>; Tue, 7 Dec 2010 23:58:40 -0800 (PST)
> Received: (from sidster@localhost)
> by mail.boxsoft.com (8.14.3/8.14.3/Submit) id oB87wdB5002430
> for tech@openbsd.org; Tue, 7 Dec 2010 23:58:40 -0800 (PST)
> X-Authentication-Warning: roppongi.boxsoft.com: sidster set sender to
> sids...@boxsoft.com using -f
> Date: Tue, 7 Dec 2010 23:58:39 -0800
> From: patrick keshishian <sids...@boxsoft.com>
> To: tech@openbsd.org
> Subject: nvi diff fixing a display glitch leading to crash
> Message-ID: <20101208075839.gc2...@roppongi.boxsoft.com>
> Mime-Version: 1.0
> Content-Type: text/plain; charset=us-ascii
> User-Agent: Mutt/1.4.2.3i
> List-Help: <mailto:majord...@openbsd.org?body=help>
> List-Owner: <mailto:tech-ow...@openbsd.org>
> List-Post: <mailto:tech@openbsd.org>
> List-Subscribe: <mailto:majord...@openbsd.org?body=sub%20tech>
> List-Unsubscribe: <mailto:majord...@openbsd.org?body=unsub%20tech>
> X-Loop: tech@openbsd.org
> Precedence: list
> Sender: owner-t...@openbsd.org
>
> Hello,
>
> I work with a lot of sources that have strange formatting
> (as do you no doubt). I noticed that sometimes changing the
> tabstop value in search of a reasonable display vi would
> crash.
>
> Took me a bit to figure out the reproducible steps to be
> able to come up with a fix.
>
> Steps to reproduce the crash. Reproducible on i386, amd64,
> and macppc:
>
> 1. Imagine a file edited with a small tabstop value (say 2
> or 3) containing lines long enough so they wrap in default
> tabstop (8), but possibly not in the smaller tabstop values.
>
> 2. While tabstop set to 8, position one of the wrapping lines
> such that the beginning of said line is off screen and the
> tail end of the wrapped portion is the first line on the
> screen with the cursor positioned on same first line.
>
> 3. Change tabstop to 2 (or 3).
> Assuming there were more lines below the line your cursor
> is on, you will notice a drawing glitch where only the
> top line is redrawn and the rest of the screen is blanked
> out.
>
> 4. If at this point you press ^B (page backwards) vi will
> core dump.
>
> A visual of what I experience (series of five screen-shots):
> http://sidster.com/scratch/nvi/
>
>
> The patch below resets the HMAP->soff (screen offset of line)
> to 1 if the number of lines HMAP->lno spans has changed and is
> now less than HMAP->soff. This essentially forces the entire
> line to be redrawn at the top of the screen, avoids the redraw
> glitch and eventual crash.
>
> Comments?
Thanks for the detailed report.
However, your diff is wrong.
> --patrick
>
>
> Index: vi/vs_smap.c
> ===================================================================
> RCS file: /cvs/obsd/src/usr.bin/vi/vi/vs_smap.c,v
> retrieving revision 1.7
> diff -u -p vi/vs_smap.c
> --- vi/vs_smap.c 27 Oct 2009 23:59:49 -0000 1.7
> +++ vi/vs_smap.c 8 Dec 2010 06:22:09 -0000
> @@ -224,6 +224,17 @@ top: HMAP->lno = lno;
> HMAP->coff = 0;
> HMAP->soff = 1;
> }
> + else {
This is not the right place to fix this, since it would affect the
other 6 calls with OOBLNO in a bad way.
> + /*
> + * If number of lines HMAP->lno (top line) spans
> + * changed due to, say reformatting, and now is
> + * fewer than HMAP->soff, reset so the line is
> + * redrawn at the top of the screen.
> + */
This should additionally check if the current line is HMAP->lno
(top line); otherwise you're changing behaviour in a strange way,
the top line will be forced to be on top. (Try it.)
> + cnt = vs_screens(sp, HMAP->lno, NULL);
This could use the cached value (cno), it would be faster for larger
lines.
> + if (cnt < HMAP->soff)
> + HMAP->soff = 1;
This should actually be cnt, not 1--you're forcing the top to be
at the beginning of the line; however this is only good for your
test case (two lines become one). This is bad for longer lines.
> + }
A similar problem applies for the TMAP case, too.
> /* If we fail, just punt. */
> for (p = HMAP, cnt = sp->t_rows; --cnt; ++p)
> if (vs_sm_next(sp, p, p + 1))
Could you verify that the attached diff fixes your problem?
Index: vi/vs_refresh.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/vi/vs_refresh.c,v
retrieving revision 1.16
diff -u -r1.16 vs_refresh.c
--- vi/vs_refresh.c 27 Oct 2009 23:59:49 -0000 1.16
+++ vi/vs_refresh.c 3 Apr 2011 10:13:47 -0000
@@ -199,9 +199,18 @@
else if (F_ISSET(sp, SC_SCR_CENTER)) {
if (vs_sm_fill(sp, LNO, P_MIDDLE))
return (1);
- } else
+ } else {
+ if (LNO == HMAP->lno || LNO == TMAP->lno) {
+ cnt = vs_screens(sp, LNO, &CNO);
+ if (LNO == HMAP->lno && cnt < HMAP->soff)
+ HMAP->soff = cnt;
+ if (LNO == TMAP->lno && cnt > TMAP->soff)
+ TMAP->soff = cnt;
+ }
+
if (vs_sm_fill(sp, OOBLNO, P_TOP))
return (1);
+ }
F_SET(sp, SC_SCR_REDRAW);
}
