mirror of
				git://git.code.sf.net/p/cdesktopenv/code
				synced 2025-03-09 15:50:02 +00:00 
			
		
		
		
	Fix command history corruption on syntax error (re: e999f6b1)
				
					
				
			Analysis: When a syntax error occurs, the shell performs a
longjmp(3) back to exfile() in main.c on line 417:
415|	if(jmpval)
416|	{
417|		Sfio_t *top;
418|		sh_iorestore((void*)shp,0,jmpval);
419|		hist_flush(shp->gd->hist_ptr);
420|		sfsync(shp->outpool);
The first thing it does is restore the file descriptor state
(sh_iorestore), then it flushes the history file (hist_flush), then
it synchronises sfio's logical stream state with the physical
stream state using (sfsync).
However, the fix applied in e999f6b1 caused sh_iorestore() to sync
all sfio streams unconditionally. So this was done before
hist_flush(), which caused unpredictable behaviour, including
temporary and/or permanent history corruption, as this also synched
shp->outpool before hist_flush() had a chance to do its thing.
The fix is to only call sfsync() in sh_iorestore() if we're
actually about to call ftruncate(2), and not otherwise.
Moral of the story: bug fixes should be as specific as possible to
minimise the risk of side effects.
src/cmd/ksh93/sh/io.c: sh_iorestore():
- Only call sfsync() if we're about to truncate a file.
src/cmd/ksh93/tests/pty.sh:
- Add test.
Thanks to Marc Wilson for reporting the bug and to Johnothan King
for finding the commit that introduced it.
Resolves: https://github.com/ksh93/ksh/issues/209
Relevant: https://github.com/att/ast/issues/61
			
			
This commit is contained in:
		
							parent
							
								
									c1986c4e1a
								
							
						
					
					
						commit
						89c69b076d
					
				
					 3 changed files with 23 additions and 6 deletions
				
			
		
							
								
								
									
										4
									
								
								NEWS
									
										
									
									
									
								
							
							
						
						
									
										4
									
								
								NEWS
									
										
									
									
									
								
							| 
						 | 
					@ -12,6 +12,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 | 
				
			||||||
- Fixed a bug introduced on 2020-08-19: Ctrl+D would break after an
 | 
					- Fixed a bug introduced on 2020-08-19: Ctrl+D would break after an
 | 
				
			||||||
  interactive shell received SIGWINCH.
 | 
					  interactive shell received SIGWINCH.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					- Fixed a bug introduced on 2020-05-21: on an interactive shell, command lines
 | 
				
			||||||
 | 
					  containing a syntax error were not added to the command history file and
 | 
				
			||||||
 | 
					  sometimes corrupted the command history.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
2021-03-05:
 | 
					2021-03-05:
 | 
				
			||||||
 | 
					
 | 
				
			||||||
- Unbalanced quotes and backticks now correctly produce a syntax error
 | 
					- Unbalanced quotes and backticks now correctly produce a syntax error
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1712,11 +1712,6 @@ void	sh_iorestore(Shell_t *shp, int last, int jmpval)
 | 
				
			||||||
	register int 	origfd, savefd, fd;
 | 
						register int 	origfd, savefd, fd;
 | 
				
			||||||
	int flag = (last&IOSUBSHELL);
 | 
						int flag = (last&IOSUBSHELL);
 | 
				
			||||||
	last &= ~IOSUBSHELL;
 | 
						last &= ~IOSUBSHELL;
 | 
				
			||||||
	/*
 | 
					 | 
				
			||||||
	 * There was an issue with truncating files (see 'ftruncate' below) that was caused by
 | 
					 | 
				
			||||||
	 * out-of-sync streams. So, to be safe, sync all streams before restoring file descriptors.
 | 
					 | 
				
			||||||
	 */
 | 
					 | 
				
			||||||
	sfsync(NULL);
 | 
					 | 
				
			||||||
	for (fd = shp->topfd - 1; fd >= last; fd--)
 | 
						for (fd = shp->topfd - 1; fd >= last; fd--)
 | 
				
			||||||
	{
 | 
						{
 | 
				
			||||||
		if(!flag && filemap[fd].subshell)
 | 
							if(!flag && filemap[fd].subshell)
 | 
				
			||||||
| 
						 | 
					@ -1740,7 +1735,10 @@ void	sh_iorestore(Shell_t *shp, int last, int jmpval)
 | 
				
			||||||
			return;
 | 
								return;
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
		if(filemap[fd].tname == Empty && shp->exitval==0)
 | 
							if(filemap[fd].tname == Empty && shp->exitval==0)
 | 
				
			||||||
 | 
							{
 | 
				
			||||||
 | 
								sfsync(NIL(Sfio_t*));
 | 
				
			||||||
			ftruncate(origfd,lseek(origfd,0,SEEK_CUR));
 | 
								ftruncate(origfd,lseek(origfd,0,SEEK_CUR));
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
		else if(filemap[fd].tname)
 | 
							else if(filemap[fd].tname)
 | 
				
			||||||
			io_usename(filemap[fd].tname,(int*)0,origfd,shp->exitval?2:1);
 | 
								io_usename(filemap[fd].tname,(int*)0,origfd,shp->exitval?2:1);
 | 
				
			||||||
		sh_close(origfd);
 | 
							sh_close(origfd);
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -107,7 +107,6 @@ if	! pty $bintrue < /dev/null
 | 
				
			||||||
then	err_exit pty command hangs on $bintrue -- tests skipped
 | 
					then	err_exit pty command hangs on $bintrue -- tests skipped
 | 
				
			||||||
	exit 0
 | 
						exit 0
 | 
				
			||||||
fi
 | 
					fi
 | 
				
			||||||
 | 
					 | 
				
			||||||
# err_exit #
 | 
					# err_exit #
 | 
				
			||||||
tst $LINENO <<"!"
 | 
					tst $LINENO <<"!"
 | 
				
			||||||
L POSIX sh 026(C)
 | 
					L POSIX sh 026(C)
 | 
				
			||||||
| 
						 | 
					@ -712,5 +711,21 @@ r ^:test-3: ls vi_completion_A_file\r\n$
 | 
				
			||||||
r ^vi_completion_A_file\r\n$
 | 
					r ^vi_completion_A_file\r\n$
 | 
				
			||||||
!
 | 
					!
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					# err_exit #
 | 
				
			||||||
 | 
					tst $LINENO <<"!"
 | 
				
			||||||
 | 
					L syntax error added to history file
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					# https://github.com/ksh93/ksh/issues/209
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					d 10
 | 
				
			||||||
 | 
					p :test-1:
 | 
				
			||||||
 | 
					w do something
 | 
				
			||||||
 | 
					r ^:test-1: do something\r\n$
 | 
				
			||||||
 | 
					r : syntax error: `do' unexpected\r\n$
 | 
				
			||||||
 | 
					w fc -lN1
 | 
				
			||||||
 | 
					r ^:test-2: fc -lN1\r\n$
 | 
				
			||||||
 | 
					r \tdo something\r\n$
 | 
				
			||||||
 | 
					!
 | 
				
			||||||
 | 
					
 | 
				
			||||||
# ======
 | 
					# ======
 | 
				
			||||||
exit $((Errors<125?Errors:125))
 | 
					exit $((Errors<125?Errors:125))
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue