csh history bug (with fix)

Tony Birnseth tonyb at daemon.UUCP
Tue Apr 30 06:57:55 AEST 1985


Index:	/usr/src/bin/csh/{sh.c,sh.hist.c,sh.lex.c}

Description:
	The original problem was from the statement in the man page 
	regarding relative history events:  
		"A history reference may be given without an event 
		specification e.g. '!$'.  In this case the reference 
		is to the previous command unless a previous history 
		reference occurred on the same line in which case this 
		form repeats the previous reference."
	Current implementation does not function that way.  Relative 
	history commands !{$,*,etc.} are always relative to the current 
	command line.

Repeat-By:
	% echo foo
	foo
	% cat /etc/printcap > /dev/null
	% !?foo?^ !$
	echo /dev/null
	/dev/null

Fix:
	Current behavior:
		The history reference is reset to the current event every 
		time a history character is encountered.  This prevents 
		relative history characters from ever referencing anything
		but the previous command line.  It became obvious, that to 
		keep the aliasing mechanism functioning for substitution of 
		"current command line arguments", Berkeley chose an 
		implementation that forces the history to be relative to the 
		current command.  e.g. 'alias echo "/bin/echo \!*"' would echo 
		its arguments.  This is accomplished by pushing the history 
		onto the history list, increment the event number, set the 
		"last event" counter equal to the current event (which hasn't 
		happened yet), then expand any history within aliases. (yuk!)  
		Unfortunately, this breaks the relative nature of the history 
		characters.  (wouldn't it have been simpler to have a modifier 
		that referenced the current line?)  
		
	Psuedo Code (for comprehension)
	Old Version:
		Get a line
			parse into words
				expand any ! or $
				if( any ! )
					set last event = current event
					(relative referencing is ALWAYS 
					relative to current event.) BUG!
				save the history 
					increment the current event. BUG!
				if(error)
					break (actually longjump)
				expand aliasing 
					if( any ! in alias itself )
						set last event=current event.
						BUG!
						(relative reference is 
						relative to next event, which 
						hasn't happened yet.)
						(This effect must be maintained)
				execute the command line
			continue

	Needed changes:
	1) Initialize the last event before any parsing occurs so that relative 
   	   history references are truly relative if the history reference is 
	   reset.  (ie !?foo?^ !$)
	2) Save the history, but do not increment the current event until after 
	   the aliasing occurs.  The current event is added to the history list.
	   This allows '^' substitution on errors.
	3) Increment the current event after aliasing occurs or if errors 
 	   are encountered.

	Scenario now looks like:
	New version:
		get a line
			set last event = current event
			parse into words
				expand any ! or $
			save history
			if(error) 
				increment current event
				break
			expand aliases
				if(any ! in alias itself)
					expand it, maintain relativity by 
					setting the next event in the history 
					stack, but don't increment the event 
					counter.
			execute the command line
			increment event number.
		continue

	Relative history is now relative.
	The aliasing behavior involving relative history is maintained.

	The changes were to:
	1) Move initialization of lastev from getexcl() in sh.lex.c to process()
   	   in sh.c.
	2) pass eventno + 1 as a parameter to enthist() from savehist() in 
	   sh.hist.c instead of ++eventno.
	3) increment eventno after alias expansion has occurred in process() 
	   in sh.c, or if an error occurred during parsing.


Tony Birnseth
===============================================================================
|	   Real			|		Virtual			      |
|	   ----			|		------- 		      |
| Small Systems Support Group	|					      |
| Computer Resource Dept.	| USENET    ../{ucbvax,decvax}!tektronix!tonyb|
| Tektronix Inc. 19-333		| CSNET     tonyb at tektronix.csnet	      |
| P.O. Box 500			| ARPAnet   tonyb%tektronix at csnet-relay.ARPA  |
| Beaverton, Or. 97077		| Ma Bell - (503) 627-5394		      |
===============================================================================

The Diffs:
Line numbers will differ.
================================================================================
RCS file: RCS/sh.c,v
Retrieving revision 1.2
diff -b  -r1.2 sh.c
770a771,776
> 		 * Initialize the last event here so that relative history
> 		 * accesses (ie !$ !*) remain relative.
> 		 */
> 		lastev = eventno;
> 
> 		/*
801c807,813
< 			error(err);
---
> 			/*
> 			 * It would be nice if we could bury eventno++ in
> 			 * error().  
> 			 * To many "uninteresting events" use error() to 
> 			 * jump back here.
> 			 */
> 			eventno++, error(err);
808c820
< 			reset();
---
> 			eventno++, reset();
817c829,839
< 			error(err);
---
> 			eventno++, error(err);
> 
> 		/*
> 		 * Increment the acutal event number here.
> 		 * If we waited till after execute(), interupted commands
> 		 * would not get their history counter incremented.
> 		 * We pad the eventno when savehist() above calls enthist() 
> 		 * to place the current command in the history list.
> 		 */
> 		if ( t && intty && !whyles ) 
> 			eventno++;
===================================================================
RCS file: RCS/sh.hist.c,v
Retrieving revision 1.2
diff -b  -r1.2 sh.hist.c
49c49
< 	enthist(++eventno, sp, 1);
---
> 	enthist(eventno + 1, sp, 1);
===================================================================
RCS file: RCS/sh.lex.c,v
Retrieving revision 1.1
diff -b  -r1.1 sh.lex.c
474c474
< 	lastev = eventno;
---
> 	/* lastev = eventno; removed from here and done in process() in sh.c */



More information about the Comp.bugs.4bsd.ucb-fixes mailing list