From 4683f4c989e90bf49fc8199d7f22c925aee836cd Mon Sep 17 00:00:00 2001
From: axel <axel@liljencrantz.se>
Date: Fri, 20 Oct 2006 01:36:03 +1000
Subject: [PATCH] Fix a number of bugs found using the warnings generated by
 earlier patch

darcs-hash:20061019153603-ac50b-4efb1ad1fe2cbe693a921648a0616a3d258d7933.gz
---
 complete.c   | 229 ++++++++++++++++++++++++++-------------------------
 exec.c       |  12 ++-
 expand.c     |   7 +-
 history.c    |   2 +-
 input.c      |   3 +-
 kill.c       |  78 ++++++++++--------
 parse_util.c |   8 +-
 parser.c     |   4 +-
 8 files changed, 188 insertions(+), 155 deletions(-)

diff --git a/complete.c b/complete.c
index 69c7748f0..eb8e3ac6e 100644
--- a/complete.c
+++ b/complete.c
@@ -909,24 +909,26 @@ static const wchar_t *complete_get_desc_suffix( const wchar_t *suff_orig )
 			array_list_t l;
 			al_init( &l );
 
-			exec_subshell( cmd, &l );
-			free(cmd);
-
-			if( al_get_count( &l )>0 )
+			if( exec_subshell( cmd, &l ) != -1 )
 			{
-				wchar_t *ln = (wchar_t *)al_get(&l, 0 );
-				if( wcscmp( ln, L"unknown" ) != 0 )
+				
+				if( al_get_count( &l )>0 )
 				{
-					desc = wcsdup( ln);
-					/*
-					  I have decided I prefer to have the description
-					  begin in uppercase and the whole universe will just
-					  have to accept it. Hah!
-					*/
-					desc[0]=towupper(desc[0]);
+					wchar_t *ln = (wchar_t *)al_get(&l, 0 );
+					if( wcscmp( ln, L"unknown" ) != 0 )
+					{
+						desc = wcsdup( ln);
+						/*
+						  I have decided I prefer to have the description
+						  begin in uppercase and the whole universe will just
+						  have to accept it. Hah!
+						*/
+						desc[0]=towupper(desc[0]);
+					}
 				}
 			}
-
+			
+			free(cmd);
 			al_foreach( &l, &free );
 			al_destroy( &l );
 		}
@@ -1155,79 +1157,82 @@ static void complete_cmd_desc( const wchar_t *cmd, array_list_t *comp )
 	  systems with a large set of manuals, but it should be ok
 	  since apropos is only called once.
 	*/
-	exec_subshell( lookup_cmd, &list );
+	if( exec_subshell( lookup_cmd, &list ) != -1 )
+	{
+		
 	
-	/*
-	  Then discard anything that is not a possible completion and put
-	  the result into a hashtable with the completion as key and the
-	  description as value.
-
-	  Should be reasonably fast, since no memory allocations are needed.
-	*/
-	for( i=0; i<al_get_count( &list); i++ )
-	{
-		wchar_t *el = (wchar_t *)al_get( &list, i );
-		wchar_t *key, *key_end, *val_begin;
-		
-		if( !el )
-			continue;
-
-		key = el+wcslen(cmd_start);
-		key_end = wcschr( el, L'\t' );
-
-		if( !key_end )
-			continue;
-		
-		*key_end = 0;
-		val_begin = key_end+1;
-		
 		/*
-		  And once again I make sure the first character is uppercased
-		  because I like it that way, and I get to decide these
-		  things.
+		  Then discard anything that is not a possible completion and put
+		  the result into a hashtable with the completion as key and the
+		  description as value.
+
+		  Should be reasonably fast, since no memory allocations are needed.
 		*/
-		val_begin[0]=towupper(val_begin[0]);
+		for( i=0; i<al_get_count( &list); i++ )
+		{
+			wchar_t *el = (wchar_t *)al_get( &list, i );
+			wchar_t *key, *key_end, *val_begin;
 		
-		hash_put( &lookup, key, val_begin );
-	}
+			if( !el )
+				continue;
 
-	/*
-	  Then do a lookup on every completion and if a match is found,
-	  change to the new description.
+			key = el+wcslen(cmd_start);
+			key_end = wcschr( el, L'\t' );
 
-	  This needs to do a reallocation for every description added, but
-	  there shouldn't be that many completions, so it should be ok.
-	*/
-	for( i=0; i<al_get_count(comp); i++ )
-	{
-		wchar_t *el = (wchar_t *)al_get( comp, i );
-		wchar_t *cmd_end = wcschr( el,
-								   COMPLETE_SEP );
-		wchar_t *new_desc;
-
-		if( cmd_end )
-			*cmd_end = 0;
-
-		new_desc = (wchar_t *)hash_get( &lookup,
-										el );
-
-		if( new_desc )
-		{
-			wchar_t *new_el = wcsdupcat2( el,
-										  COMPLETE_SEP_STR,
-										  new_desc,
-										  (void *)0 );
-
-			al_set( comp, i, new_el );
-			free( el );
+			if( !key_end )
+				continue;
+		
+			*key_end = 0;
+			val_begin = key_end+1;
+		
+			/*
+			  And once again I make sure the first character is uppercased
+			  because I like it that way, and I get to decide these
+			  things.
+			*/
+			val_begin[0]=towupper(val_begin[0]);
+		
+			hash_put( &lookup, key, val_begin );
 		}
-		else
+
+		/*
+		  Then do a lookup on every completion and if a match is found,
+		  change to the new description.
+
+		  This needs to do a reallocation for every description added, but
+		  there shouldn't be that many completions, so it should be ok.
+		*/
+		for( i=0; i<al_get_count(comp); i++ )
 		{
+			wchar_t *el = (wchar_t *)al_get( comp, i );
+			wchar_t *cmd_end = wcschr( el,
+									   COMPLETE_SEP );
+			wchar_t *new_desc;
+
 			if( cmd_end )
-				*cmd_end = COMPLETE_SEP;
+				*cmd_end = 0;
+
+			new_desc = (wchar_t *)hash_get( &lookup,
+											el );
+
+			if( new_desc )
+			{
+				wchar_t *new_el = wcsdupcat2( el,
+											  COMPLETE_SEP_STR,
+											  new_desc,
+											  (void *)0 );
+
+				al_set( comp, i, new_el );
+				free( el );
+			}
+			else
+			{
+				if( cmd_end )
+					*cmd_end = COMPLETE_SEP;
+			}
 		}
 	}
-
+	
 	hash_destroy( &lookup );
 	al_foreach( &list,
 				&free );
@@ -1448,29 +1453,29 @@ static void complete_from_args( const wchar_t *str,
 	  strings should be escaped!
 	*/
 	/*
-	for( i=0; i< al_get_count( &possible_comp ); i++ )
-	{
-		wchar_t *next = (wchar_t *)al_get( &possible_comp, i );
-		wchar_t *next_unescaped;
-		if( next )
-		{
-			next_unescaped = unescape( next, 0 );
-			if( next_unescaped )
-			{
-				al_set( &possible_comp , i, next_unescaped );
-			}
-			else
-			{
-				al_set( &possible_comp , i, 0 );
-				debug( 2, L"Could not expand string %ls on line %d of file %s", next, __LINE__, __FILE__ );
-			}
-			free( next );
-		}
-		else
-		{
-			debug( 2, L"Got null string on line %d of file %s", __LINE__, __FILE__ );
-		}
-	}
+	  for( i=0; i< al_get_count( &possible_comp ); i++ )
+	  {
+	  wchar_t *next = (wchar_t *)al_get( &possible_comp, i );
+	  wchar_t *next_unescaped;
+	  if( next )
+	  {
+	  next_unescaped = unescape( next, 0 );
+	  if( next_unescaped )
+	  {
+	  al_set( &possible_comp , i, next_unescaped );
+	  }
+	  else
+	  {
+	  al_set( &possible_comp , i, 0 );
+	  debug( 2, L"Could not expand string %ls on line %d of file %s", next, __LINE__, __FILE__ );
+	  }
+	  free( next );
+	  }
+	  else
+	  {
+	  debug( 2, L"Got null string on line %d of file %s", __LINE__, __FILE__ );
+	  }
+	  }
 	*/
 
 	copy_strings_with_prefix( comp_out, str, desc, 0, &possible_comp );
@@ -1789,7 +1794,7 @@ static void complete_param_expand( wchar_t *str,
 								   int do_file )
 {
 	wchar_t *comp_str;
-
+	
 	if( (wcsncmp( str, L"--", 2 )) == 0 && (comp_str = wcschr(str, L'=' ) ) )
 	{
 		comp_str++;
@@ -1798,16 +1803,18 @@ static void complete_param_expand( wchar_t *str,
 	{
 		comp_str = str;
 	}
-	/*
-	debug( 3,
-		   L"expand_string( \"%ls\", comp_out, EXPAND_SKIP_CMDSUBST | ACCEPT_INCOMPLETE | %ls );",
-		   comp_str,
-		   do_file?L"0":L"EXPAND_SKIP_WILDCARDS" );
-	*/
-	expand_string( 0, 
-				   wcsdup(comp_str),
-				   comp_out,
-				   EXPAND_SKIP_CMDSUBST | ACCEPT_INCOMPLETE | (do_file?0:EXPAND_SKIP_WILDCARDS) );
+
+	if( expand_string( 0, 
+					   wcsdup(comp_str),
+					   comp_out,
+					   EXPAND_SKIP_CMDSUBST | ACCEPT_INCOMPLETE | (do_file?0:EXPAND_SKIP_WILDCARDS) ) )
+	{
+		/*
+		  Ignore errors - completions may fail, and that's ok
+		*/
+	}
+	
+	
 }
 
 /**
@@ -1974,7 +1981,7 @@ static int try_complete_user( const wchar_t *cmd,
 														 L"/",
 														 COMPLETE_SEP_STR,
 														 COMPLETE_USER_DESC,
-														 0 );
+														 (void *)0 );
 							if( blarg != 0 )
 							{
 								al_push( comp, blarg );
diff --git a/exec.c b/exec.c
index bd4440bd9..501369b32 100644
--- a/exec.c
+++ b/exec.c
@@ -1344,7 +1344,7 @@ int exec_subshell( const wchar_t *cmd,
 		debug( 1, 
 			   _( L"Sent null command to subshell. This is a fish bug. If it can be reproduced, please send a bug report to %s." ), 
 			   PACKAGE_BUGREPORT );		
-		return 0;		
+		return -1;		
 	}
 	
 	is_subshell=1;	
@@ -1352,11 +1352,17 @@ int exec_subshell( const wchar_t *cmd,
 	
 	prev_status = proc_get_last_status();
 	
-	eval( cmd, io_buffer, SUBST );
+	if( eval( cmd, io_buffer, SUBST ) )
+	{
+		status = -1;
+	}
+	else
+	{
+		status = proc_get_last_status();
+	}
 	
 	io_buffer_read( io_buffer );
 	
-	status = proc_get_last_status();
 	proc_set_last_status( prev_status );
 	
 	is_subshell = prev_subshell;
diff --git a/expand.c b/expand.c
index 87a6b2064..2235efe92 100644
--- a/expand.c
+++ b/expand.c
@@ -1265,7 +1265,12 @@ static int expand_cmdsubst( wchar_t *in, array_list_t *out )
 	wcslcpy( subcmd, paran_begin+1, paran_end-paran_begin );
 	subcmd[ paran_end-paran_begin-1]=0;
 
-	exec_subshell( subcmd, sub_res);
+	if( exec_subshell( subcmd, sub_res) == -1 )
+	{
+		halloc_free( context );
+		error( CMDSUBST_ERROR, -1, L"Unknown error while evaulating command substitution" );
+		return 0;
+	}
 
 	tail_begin = paran_end + 1;
 	if( *tail_begin == L'[' )
diff --git a/history.c b/history.c
index f98ddaff6..96bf42053 100644
--- a/history.c
+++ b/history.c
@@ -243,7 +243,7 @@ static wchar_t *history_file_name( void *context )
 	halloc_register_function( context, &free, res );
 	return res;
 }
-		
+
 /**
    Read a complete histor entry from the specified FILE.
 */
diff --git a/input.c b/input.c
index 406e4b2e2..19a4cded1 100644
--- a/input.c
+++ b/input.c
@@ -1297,6 +1297,7 @@ static void add_common_bindings()
 		add_escaped_mapping( name[i], (L"\\C-y"), L"Control-y", L"yank" );
 		add_mapping( name[i], L"", L"Any key", L"self-insert" );
 	}		
+
 }
 
 /**
@@ -1421,7 +1422,7 @@ int input_init()
 	is_init = 1;
 
 	input_common_init( &interrupt_handler );
-	
+
 	if( setupterm( 0, STDOUT_FILENO, 0) == ERR )
 	{
 		debug( 0, _( L"Could not set up terminal" ) );
diff --git a/kill.c b/kill.c
index ff480a0ae..813381012 100644
--- a/kill.c
+++ b/kill.c
@@ -97,7 +97,13 @@ void kill_add( wchar_t *str )
 	{
 		wchar_t *escaped_str = escape( str, 1 );
 		wchar_t *cmd = wcsdupcat2(L"echo ", escaped_str, L"|xsel -b",(void *)0);
-		exec_subshell( cmd, 0 );
+		if( exec_subshell( cmd, 0 ) == -1 )
+		{
+			/* 
+			   Do nothing on failiure
+			*/
+		}
+		
 		free( cut_buffer );
 		free( cmd );
 	
@@ -192,49 +198,51 @@ static void kill_check_x_buffer()
 		wchar_t *new_cut_buffer=0;
 		array_list_t list;
 		al_init( &list );
-		exec_subshell( cmd, &list );
-		
-		for( i=0; i<al_get_count( &list ); i++ )
+		if( exec_subshell( cmd, &list ) != -1 )
 		{
-			wchar_t *next_line = escape( (wchar_t *)al_get( &list, i ), 0 );
-			if( i==0 )
+					
+			for( i=0; i<al_get_count( &list ); i++ )
 			{
-				new_cut_buffer = next_line;
-			}
-			else
-			{
-				wchar_t *old = new_cut_buffer;
-				new_cut_buffer= wcsdupcat2( new_cut_buffer, L"\\n", next_line, (void *)0 );
-				free( old );
-				free( next_line );	
-			}
-		}
-		
-		if( new_cut_buffer )
-		{
-			/*  
-				The buffer is inserted with backslash escapes,
-				since we don't really like tabs, newlines,
-				etc. anyway.
-			*/
-			
-			if( cut_buffer != 0 )
-			{      
-				if( wcscmp( new_cut_buffer, cut_buffer ) == 0 )
+				wchar_t *next_line = escape( (wchar_t *)al_get( &list, i ), 0 );
+				if( i==0 )
 				{
-					free( new_cut_buffer );
-					new_cut_buffer = 0;
+					new_cut_buffer = next_line;
 				}
 				else
 				{
-					free( cut_buffer );
-					cut_buffer = 0;								
+					wchar_t *old = new_cut_buffer;
+					new_cut_buffer= wcsdupcat2( new_cut_buffer, L"\\n", next_line, (void *)0 );
+					free( old );
+					free( next_line );	
 				}
 			}
-			if( cut_buffer == 0 )
+		
+			if( new_cut_buffer )
 			{
-				cut_buffer = new_cut_buffer;
-				kill_add_internal( cut_buffer );
+				/*  
+					The buffer is inserted with backslash escapes,
+					since we don't really like tabs, newlines,
+					etc. anyway.
+				*/
+				
+				if( cut_buffer != 0 )
+				{      
+					if( wcscmp( new_cut_buffer, cut_buffer ) == 0 )
+					{
+						free( new_cut_buffer );
+						new_cut_buffer = 0;
+					}
+					else
+					{
+						free( cut_buffer );
+						cut_buffer = 0;								
+				}
+				}
+				if( cut_buffer == 0 )
+				{
+					cut_buffer = new_cut_buffer;
+					kill_add_internal( cut_buffer );
+				}
 			}
 		}
 		
diff --git a/parse_util.c b/parse_util.c
index 7a01af9df..dc3ae9a5f 100644
--- a/parse_util.c
+++ b/parse_util.c
@@ -878,7 +878,13 @@ static int parse_util_load_internal( const wchar_t *cmd,
 				/*
 				  Source the completion file for the specified completion
 				*/
-				exec_subshell( src_cmd, 0 );
+				if( exec_subshell( src_cmd, 0 ) == -1 )
+				{
+					/*
+					  Do nothing on failiure
+					*/
+				}
+				
 				free(src_cmd);
 				reloaded = 1;
 			}
diff --git a/parser.c b/parser.c
index 9d0f38b30..588ca4644 100644
--- a/parser.c
+++ b/parser.c
@@ -2910,7 +2910,7 @@ int parser_test( const  wchar_t * buff,
 						*/
 						if( contains_str( cmd,
 										  L"end",
-										  0 ) )
+										  (void *)0 ) )
 						{
 							err=1;
 							if( out )
@@ -2985,7 +2985,7 @@ int parser_test( const  wchar_t * buff,
 					if( contains_str( cmd,
 									  L"or",
 									  L"and",
-									  0 ) )
+									  (void *)0 ) )
 					{
 						/*
 						  'or' and 'and' can not be used inside pipelines