bin/21089: vi silently corrupt open file on SIGINT when entering :wq in command mode

[Yar Tikhiy]

The following reply was made to PR bin/21089; it has been noted by GNATS.

From: Yar Tikhiy <yar at comp.chem.msu.su>
To: Jaakko Heinonen <jh at saunalahti.fi>
Cc: bug-followup at freebsd.org
Subject: Re: bin/21089: vi silently corrupt open file on SIGINT when entering :wq in command mode
Date: Sun, 10 Feb 2008 01:12:44 +0300

 Jaakko,
 
 I think that you looked in the right direction but applied the fix
 at a wrong level.  In fact, we shouldn't put a test for INTERRUPTED(sp)
 into each ex command's implementation.  In ex mode, the call stack
 is as follows:
 
 	main -> editor -> ex -> ex_cmd -> <individual command>
 
 Among the functions, it's ex() that takes care of INTERRUPTED(sp).
 Therefore we should do something similar, too, when ex_cmd() is called
 from vi mode.  In the latter case the call stack looks as follows:
 
 	main -> editor -> vi -> v_ex -> ex_cmd -> <individual command>
 
 So we should be looking at somewhere around v_ex().  But, in fact,
 a similar problem is found in all consumers of v_tcmd(): ^C doesn't
 abort the command being typed, it commits the command instead.  That
 applies both to colon commands and to search commands.  In the case
 of `:w' the problem just gets much worse because interrupted status
 remains set and blows up the command later.
 
 To make things even more complicated, a window resize event behaves
 much like ^C in that it commits the command being typed, which can
 have an unwanted result.
 
 The NetBSD folks seem to have fixed this bug generally in v_ex.c
 and v_text.c, but their fix is hackish, too: they now pretend that
 ^C or a window resize event is the same as <Esc>.
 
 What perhaps is needed is a clear indication from v_txt() to its
 caller that a special event ended text input, so that it can be up
 to the caller to decide how to handle the event.
 
 But for now the ^C bug can be fixed quickly as shown in the attached
 patch.  With the patch applied, ^C will commit neither ex commands
 nor search commands: it'll abort them instead, which makes much more
 sense IMHO.
 
 Yar
 
 Index: vi/v_txt.c
 ===================================================================
 RCS file: /home/ncvs/src/contrib/nvi/vi/v_txt.c,v
 retrieving revision 1.1.1.1
 diff -u -p -r1.1.1.1 v_txt.c
 --- vi/v_txt.c	1 Nov 1996 06:45:33 -0000	1.1.1.1
 +++ vi/v_txt.c	9 Feb 2008 22:06:02 -0000
 @@ -110,6 +110,10 @@ v_tcmd(sp, vp, prompt, flags)
  	sp->lno = vp->m_final.lno;
  	sp->cno = vp->m_final.cno;
  
 +	/* Interrupt aborts the command. */
 +	if (INTERRUPTED(sp))
 +		return (1);
 +
  	return (0);
  }