Index | Thread | Search

From:
Walter Alejandro Iglesias <wai@roquesor.com>
Subject:
Re: vi(1), adding sizeof() to malloc when using CHAR_T
To:
Theo Buehler <tb@theobuehler.org>
Cc:
Otto Moerbeek <otto@drijf.net>, tech@openbsd.org
Date:
Mon, 20 Apr 2026 14:17:59 +0200

Download raw body.

Thread
On Mon, Apr 20, 2026 at 01:20:34PM +0200, Walter Alejandro Iglesias wrote:
> On Mon, Apr 20, 2026 at 01:04:19PM +0200, Theo Buehler wrote:
> > On Mon, Apr 20, 2026 at 12:53:00PM +0200, Otto Moerbeek wrote:
> > > On Mon, Apr 20, 2026 at 12:17:16PM +0200, Walter Alejandro Iglesias wrote:
> > > 
> > > > Hunting the cause of this bug in our vi(1):
> > > > 
> > > >   https://marc.info/?l=openbsd-misc&m=177649923622612&w=2
> > > > 
> > > > I compiled nvi2 from ports on linux to be able to use valgrind.  The
> > > > output at bottom shows a bunch "Invalid write" that happens after
> > > > removing the '* sizeof(CHAR_T)' from MALLOC_RET (which is already added
> > > > in this version) in ex_g_setup() (ex/ex_global.c:172).  The diff below
> > > > adds that same multiplication in other places where CHAR_T is used.
> > > > 
> > > > I don't know if this affects OpenBSD's malloc in the same manner, but I
> > > > guess it doesn't hurt to add it:
> > > 
> > > For OpenBSD CHAR_T is uchar_t, which is 1 by definition. So I'm not
> > > sure this makes sense as long as we keep it that way.
> > 
> > Well, as the diff shows, the code is rather confused about this.
> > 
> > 	MALLOC(sp, copy, len + 1);
> > 	if (copy == NULL)
> > 		return (NULL);
> > 	memcpy(copy, str, len * sizeof(CHAR_T));
> > 	copy[len] = '\0';
> > 
> > So just modifying the MALLOC lines doesn't really improve things anyway.
> > Arguably, it would be beneficial to fix this one way or the other.
> 
> nvi2 from ports and debian 1.81.6, since they have added UTF-8 support,
> have two functions, one for char and one for wide char:

Relevant part I missed, sorry!

In nvi2 (common/multibyte.h):

  #ifdef USE_WIDECHAR
  ...
  typedef	wchar_t		CHAR_T;
  #else
  typedef	u_char		CHAR_T;

In our vi (common/key.h):

  typedef	u_char		CHAR_T;


-- 
Walter