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

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.