Index | Thread | Search

From:
Walter Alejandro Iglesias <wai@roquesor.com>
Subject:
Re: This is indeed a bug (ex Re: vi E_CLRFLAG not being used correctly?)
To:
tech@openbsd.org
Cc:
Jeremy Mates <jmates@thrig.me>
Date:
Tue, 19 May 2026 10:47:31 +0200

Download raw body.

Thread
Ping.

On Wed, Feb 04, 2026 at 03:39:39PM +0100, Walter Alejandro Iglesias wrote:
> On Sat, Jan 31, 2026 at 12:59:35AM +0000, Jeremy Mates wrote:
> > 	$ printf 'append\na\nb\nc\n.\nnumber\nnumber l\nq!\n' | ex         
> > 		 3  c
> > 		 3  c$
> > 	c$
> > 
> > In ex mode, the "number" command when given the "l" (literal display)
> > flag causes an additional line to be printed by the autoprint code in
> > ex/ex.c. The autoprint code tries to turn off the various flags when
> > E_CLRFLAG is set by an ex command. However, it appears that E_CLRFLAG is
> > put into ecp->flags ("current flags") from the command flags,
> > 
> >     /* Add standard command flags. */
> >     F_SET(ecp, ecp->cmd->flags);
> > 
> > while the ex/ex.c autoprint code instead checks ecp->iflags ("User input
> > information") for E_CLRFLAG, hence the "number l" command causing an
> > additional ex_print line, as E_C_LIST is not turned off.
> > 
> >         /*
> >          * The print commands have already handled the `print' flags.
> >          * If so, clear them.
> >          */
> >         if (FL_ISSET(ecp->iflags, E_CLRFLAG))
> >             FL_CLR(ecp->iflags, E_C_HASH | E_C_LIST | E_C_PRINT);
> > 
> > A fix might be to use ->flags (where E_CLRFLAG is) instead of ->iflags?
> > 
> > --- ex/ex.c
> > +++ ex/ex.c
> > @@ -1439,7 +1439,7 @@ addr_verify:
> >  		 * The print commands have already handled the `print' flags.
> >  		 * If so, clear them.
> >  		 */
> > -		if (FL_ISSET(ecp->iflags, E_CLRFLAG))
> > +		if (FL_ISSET(ecp->flags, E_CLRFLAG))
> >  			FL_CLR(ecp->iflags, E_C_HASH | E_C_LIST | E_C_PRINT);
> >  
> >  		/* If hash set only because of the number option, discard it. */
> > 
> > 
> 
> 
> The E_CLRFLAG is defined in ex/ex.h as hexadecimal '80':
> 
>   $ grep -Ir /usr/src/usr.bin/vi '^#define.*E_CLRFLAG'
>   ex/ex.h:#define E_CLRFLAG       0x00000080      /* Clear the print (#, l, p) flags. */
> 
> If you compile vi(1) after appling this test diff:
> 
> Index: ex/ex.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/ex/ex.c,v
> diff -u -p -u -p -r1.23 ex.c
> --- ex/ex.c	23 Jun 2023 15:06:45 -0000	1.23
> +++ ex/ex.c	4 Feb 2026 14:02:49 -0000
> @@ -1439,7 +1439,17 @@ addr_verify:
>  		 * The print commands have already handled the `print' flags.
>  		 * If so, clear them.
>  		 */
> -		if (FL_ISSET(ecp->iflags, E_CLRFLAG))
> +
> +		/* -------------- TEST -------------------------- */
> +		printf("F_ISSET(ecp, E_CLRFLAG) = %x\n",
> +		    F_ISSET(ecp, E_CLRFLAG));
> +		printf("FL_ISSET(ecp->iflags, E_CLRFLAG) = %x\n",
> +		    FL_ISSET(ecp->iflags, E_CLRFLAG));
> +		printf("FL_ISSET(ecp->flags, E_CLRFLAG) = %x\n",
> +		    FL_ISSET(ecp->flags, E_CLRFLAG));
> +		/* ------------------------------------------------ */
> +
> +		if (F_ISSET(ecp, E_CLRFLAG))
>  			FL_CLR(ecp->iflags, E_C_HASH | E_C_LIST | E_C_PRINT);
>  
>  		/* If hash set only because of the number option, discard it. */
> 
> 
> And then you run:
> 
>   (You can replace the command 'l' for '#', 'p' or any
>    combination of them)
>                       \
>   $ printf 'i\nfoo\n.\nl\nq!\n' | ex
>   F_ISSET(ecp, E_CLRFLAG) = 0
>   FL_ISSET(ecp->iflags, E_CLRFLAG) = 0
>   FL_ISSET(ecp->flags, E_CLRFLAG) = 0
>   foo$
>   F_ISSET(ecp, E_CLRFLAG) = 80
>   FL_ISSET(ecp->iflags, E_CLRFLAG) = 0
>   FL_ISSET(ecp->flags, E_CLRFLAG) = 80
>   F_ISSET(ecp, E_CLRFLAG) = 0
>   FL_ISSET(ecp->iflags, E_CLRFLAG) = 0
>   FL_ISSET(ecp->flags, E_CLRFLAG) = 0
> 
> You see that from the three conditions (I added a third one[1])
> this one isn't useful:
> 
>   FL_ISSET(ecp->iflags, E_CLRFLAG) = 0
> 
> This confirms that what Jeremy found is indeed a bug and his proposed
> diff fixes it.  Below I include a optional diff[1] using as condition
> F_ISSET(ecp, E_CLRFLAG) which, in my oppinion, follows the idiom of the
> rest of the code.
> 
> 
> Index: ex/ex.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/ex/ex.c,v
> diff -u -p -u -p -r1.23 ex.c
> --- ex/ex.c	23 Jun 2023 15:06:45 -0000	1.23
> +++ ex/ex.c	4 Feb 2026 14:19:26 -0000
> @@ -1439,7 +1439,7 @@ addr_verify:
>  		 * The print commands have already handled the `print' flags.
>  		 * If so, clear them.
>  		 */
> -		if (FL_ISSET(ecp->iflags, E_CLRFLAG))
> +		if (F_ISSET(ecp, E_CLRFLAG))
>  			FL_CLR(ecp->iflags, E_C_HASH | E_C_LIST | E_C_PRINT);
>  
>  		/* If hash set only because of the number option, discard it. */
> 
> 


Index: ex/ex.c
===================================================================
RCS file: /cvs/src/usr.bin/vi/ex/ex.c,v
diff -u -p -u -p -r1.23 ex.c
--- ex/ex.c	23 Jun 2023 15:06:45 -0000	1.23
+++ ex/ex.c	4 Feb 2026 14:19:26 -0000
@@ -1439,7 +1439,7 @@ addr_verify:
 		 * The print commands have already handled the `print' flags.
 		 * If so, clear them.
 		 */
-		if (FL_ISSET(ecp->iflags, E_CLRFLAG))
+		if (F_ISSET(ecp, E_CLRFLAG))
 			FL_CLR(ecp->iflags, E_C_HASH | E_C_LIST | E_C_PRINT);
 
 		/* If hash set only because of the number option, discard it. */



-- 
Walter