Re: vi(1) filter command at EOF causes data corruption

The diff below includes a change that shouldn't be there.  The following
is wrong:

  	/* Flush the cache, update line count, before screen update. */
  -	if (lno >= ep->c_lno)
  +	if (lno <= ep->c_lno)


That's from something I wrongly changed at some point and forgot to
undo, and slipped in the final diff.  At bottom I include the updated
version of this diff.


On Fri, May 22, 2026 at 09:26:00AM +0200, Walter Alejandro Iglesias wrote:
> DEEPER SOLUTION:
> 
> The diff at bottom is extensive, but a deeper solution.  It uses realloc
> and memcpy to create a private copy of the data.  This is somewhat
> closer to what nvi2 from ports does, where this bug is fixed
> (accidentally?) by this intermediate translation layer for wide chars
> support:
> 
> 	if (FILE2INT(sp, data.data, data.size, wp, wlen)) {
> 	    if (!F_ISSET(sp, SC_CONV_ERROR)) {
> 		F_SET(sp, SC_CONV_ERROR);
> 		msgq(sp, M_ERR, "324|Conversion error on line %d", lno);
> 	    }
> 	    goto err3;
> 	}
> 
> 	/* Reset the cache. */
> 	if (wp != data.data) {
> 	    BINC_GOTOW(sp, sp->c_lp, sp->c_blen, wlen);
> 	    MEMCPYW(sp->c_lp, wp, wlen);
> 	}
> 	sp->c_lno = lno;
> 	sp->c_len = wlen;
> 
> 
> Index: common/exf.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/common/exf.c,v
> diff -u -p -u -p -r1.51 exf.c
> --- common/exf.c	20 Apr 2026 10:30:02 -0000	1.51
> +++ common/exf.c	4 May 2026 13:25:07 -0000
> @@ -434,6 +434,11 @@ oerr:	if (F_ISSET(ep, F_RCV_ON))
>  	ep->rcv_path = NULL;
>  	if (ep->db != NULL)
>  		(void)ep->db->close(ep->db);
> +
> +	/* Free the private cache buffer. */
> +	if (ep->c_buf != NULL)
> +		free(ep->c_buf);
> +
>  	free(ep);
>  
>  	return (open_err ?
> @@ -715,6 +720,11 @@ file_end(SCR *sp, EXF *ep, int force)
>  		(void)close(ep->rcv_fd);
>  	free(ep->rcv_path);
>  	free(ep->rcv_mpath);
> +
> +	/* Free the private cache buffer. */
> +	if (ep->c_buf != NULL)
> +		free(ep->c_buf);
> +
>  	free(ep);
>  	return (0);
>  }
> Index: common/exf.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/common/exf.h,v
> diff -u -p -u -p -r1.6 exf.h
> --- common/exf.h	20 Feb 2022 19:45:51 -0000	1.6
> +++ common/exf.h	4 May 2026 13:25:07 -0000
> @@ -24,7 +24,8 @@ struct _exf {
>  	size_t	 c_len;			/* Cached line length. */
>  	recno_t	 c_lno;			/* Cached line number. */
>  	recno_t	 c_nlines;		/* Cached lines in the file. */
> -
> +	char	*c_buf;			/* Buffer used to reset cache */
> +	size_t	 c_buf_len;		/* Length of buffer used to reset cache */
>  	DB	*log;			/* Log db structure. */
>  	char	*l_lp;			/* Log buffer. */
>  	size_t	 l_len;			/* Log buffer length. */
> Index: common/line.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/common/line.c,v
> diff -u -p -u -p -r1.18 line.c
> --- common/line.c	20 Apr 2026 10:30:02 -0000	1.18
> +++ common/line.c	4 May 2026 13:25:07 -0000
> @@ -19,6 +19,7 @@
>  #include <errno.h>
>  #include <limits.h>
>  #include <stdio.h>
> +#include <stdlib.h>
>  #include <string.h>
>  
>  #include "common.h"
> @@ -156,9 +157,8 @@ err3:		if (lenp != NULL)
>  	}
>  
>  	/* Reset the cache. */
> -	ep->c_lno = lno;
> -	ep->c_len = data.size;
> -	ep->c_lp = data.data;
> +	if (db_cache_update(sp, ep, lno, data.data, data.size))
> +		goto err3;
>  
>  #if defined(DEBUG) && 0
>  	TRACE(sp, "retrieve DB line %lu\n", (u_long)lno);
> @@ -326,7 +326,7 @@ db_insert(SCR *sp, recno_t lno, char *p,
>  	}
>  
>  	/* Flush the cache, update line count, before screen update. */
> -	if (lno >= ep->c_lno)
> +	if (lno <= ep->c_lno)
>  		ep->c_lno = OOBLNO;
>  	if (ep->c_nlines != OOBLNO)
>  		++ep->c_nlines;
> @@ -484,9 +484,9 @@ db_last(SCR *sp, recno_t *lnop)
>  
>  	/* Fill the cache. */
>  	memcpy(&lno, key.data, sizeof(lno));
> +	if (db_cache_update(sp, ep, lno, data.data, data.size))
> +		return (1);
>  	ep->c_nlines = ep->c_lno = lno;
> -	ep->c_len = data.size;
> -	ep->c_lp = data.data;
>  
>  	/* Return the value. */
>  	*lnop = (F_ISSET(sp, SC_TINPUT) &&
> @@ -529,4 +529,30 @@ scr_update(SCR *sp, recno_t lno, lnop_t 
>  				if (vs_change(tsp, lno, op))
>  					return (1);
>  	return (current ? vs_change(sp, lno, op) : 0);
> +}
> +
> +/*
> + * db_cache_update --
> + *	Update the line cache with a private copy of the data.
> + */
> +int
> +db_cache_update(SCR *sp, EXF *ep, recno_t lno, void *data, size_t size)
> +{
> +	if (size > ep->c_buf_len) {
> +		REALLOC(sp, ep->c_buf, size);
> +		if (ep->c_buf == NULL) {
> +			ep->c_buf_len = 0;
> +			ep->c_lp = NULL;
> +			ep->c_lno = OOBLNO;
> +			return (1);
> +		}
> +		ep->c_buf_len = size;
> +	}
> +	if (size > 0)
> +		memcpy(ep->c_buf, data, size);
> +
> +	ep->c_lno = lno;
> +	ep->c_len = size;
> +	ep->c_lp = ep->c_buf;
> +	return (0);
>  }
> Index: include/com_extern.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/include/com_extern.h,v
> diff -u -p -u -p -r1.17 com_extern.h
> --- include/com_extern.h	23 Aug 2025 21:02:10 -0000	1.17
> +++ include/com_extern.h	4 May 2026 13:25:07 -0000
> @@ -34,6 +34,7 @@ int db_insert(SCR *, recno_t, char *, si
>  int db_set(SCR *, recno_t, char *, size_t);
>  int db_exist(SCR *, recno_t);
>  int db_last(SCR *, recno_t *);
> +int db_cache_update(SCR *, EXF *, recno_t, void *, size_t);
>  void db_err(SCR *, recno_t);
>  int log_init(SCR *, EXF *);
>  int log_end(SCR *, EXF *);
> 
> 


Corrected version:

Index: common/exf.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/common/exf.c,v
diff -u -p -u -p -r1.51 exf.c
--- common/exf.c	20 Apr 2026 10:30:02 -0000	1.51
+++ common/exf.c	17 Jun 2026 12:21:11 -0000
@@ -434,6 +434,11 @@ oerr:	if (F_ISSET(ep, F_RCV_ON))
 	ep->rcv_path = NULL;
 	if (ep->db != NULL)
 		(void)ep->db->close(ep->db);
+
+	/* Free the private cache buffer. */
+	if (ep->c_buf != NULL)
+		free(ep->c_buf);
+
 	free(ep);
 
 	return (open_err ?
@@ -715,6 +720,11 @@ file_end(SCR *sp, EXF *ep, int force)
 		(void)close(ep->rcv_fd);
 	free(ep->rcv_path);
 	free(ep->rcv_mpath);
+
+	/* Free the private cache buffer. */
+	if (ep->c_buf != NULL)
+		free(ep->c_buf);
+
 	free(ep);
 	return (0);
 }
Index: common/exf.h
===================================================================
RCS file: /cvs/src/usr.bin/vi/common/exf.h,v
diff -u -p -u -p -r1.6 exf.h
--- common/exf.h	20 Feb 2022 19:45:51 -0000	1.6
+++ common/exf.h	17 Jun 2026 12:21:11 -0000
@@ -24,7 +24,8 @@ struct _exf {
 	size_t	 c_len;			/* Cached line length. */
 	recno_t	 c_lno;			/* Cached line number. */
 	recno_t	 c_nlines;		/* Cached lines in the file. */
-
+	char	*c_buf;			/* Buffer used to reset cache */
+	size_t	 c_buf_len;		/* Length of buffer used to reset cache */
 	DB	*log;			/* Log db structure. */
 	char	*l_lp;			/* Log buffer. */
 	size_t	 l_len;			/* Log buffer length. */
Index: common/line.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/common/line.c,v
diff -u -p -u -p -r1.18 line.c
--- common/line.c	20 Apr 2026 10:30:02 -0000	1.18
+++ common/line.c	17 Jun 2026 12:21:11 -0000
@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include "common.h"
@@ -156,9 +157,8 @@ err3:		if (lenp != NULL)
 	}
 
 	/* Reset the cache. */
-	ep->c_lno = lno;
-	ep->c_len = data.size;
-	ep->c_lp = data.data;
+	if (db_cache_update(sp, ep, lno, data.data, data.size))
+		goto err3;
 
 #if defined(DEBUG) && 0
 	TRACE(sp, "retrieve DB line %lu\n", (u_long)lno);
@@ -484,9 +484,9 @@ db_last(SCR *sp, recno_t *lnop)
 
 	/* Fill the cache. */
 	memcpy(&lno, key.data, sizeof(lno));
+	if (db_cache_update(sp, ep, lno, data.data, data.size))
+		return (1);
 	ep->c_nlines = ep->c_lno = lno;
-	ep->c_len = data.size;
-	ep->c_lp = data.data;
 
 	/* Return the value. */
 	*lnop = (F_ISSET(sp, SC_TINPUT) &&
@@ -529,4 +529,30 @@ scr_update(SCR *sp, recno_t lno, lnop_t 
 				if (vs_change(tsp, lno, op))
 					return (1);
 	return (current ? vs_change(sp, lno, op) : 0);
+}
+
+/*
+ * db_cache_update --
+ *	Update the line cache with a private copy of the data.
+ */
+int
+db_cache_update(SCR *sp, EXF *ep, recno_t lno, void *data, size_t size)
+{
+	if (size > ep->c_buf_len) {
+		REALLOC(sp, ep->c_buf, size);
+		if (ep->c_buf == NULL) {
+			ep->c_buf_len = 0;
+			ep->c_lp = NULL;
+			ep->c_lno = OOBLNO;
+			return (1);
+		}
+		ep->c_buf_len = size;
+	}
+	if (size > 0)
+		memcpy(ep->c_buf, data, size);
+
+	ep->c_lno = lno;
+	ep->c_len = size;
+	ep->c_lp = ep->c_buf;
+	return (0);
 }
Index: include/com_extern.h
===================================================================
RCS file: /cvs/src/usr.bin/vi/include/com_extern.h,v
diff -u -p -u -p -r1.17 com_extern.h
--- include/com_extern.h	23 Aug 2025 21:02:10 -0000	1.17
+++ include/com_extern.h	17 Jun 2026 12:21:11 -0000
@@ -34,6 +34,7 @@ int db_insert(SCR *, recno_t, char *, si
 int db_set(SCR *, recno_t, char *, size_t);
 int db_exist(SCR *, recno_t);
 int db_last(SCR *, recno_t *);
+int db_cache_update(SCR *, EXF *, recno_t, void *, size_t);
 void db_err(SCR *, recno_t);
 int log_init(SCR *, EXF *);
 int log_end(SCR *, EXF *);



-- 
Walter