Change parser_t::parse_job's args array to be a local, rather than just leak it, which no longer seems to crash

7 files changed, 43 insertions, 41 deletions

diff --git a/builtin.cpp b/builtin.cpp
index cca47249..84f563ac 100644
--- a/builtin.cpp
+++ b/builtin.cpp
@@ -2599,7 +2599,6 @@ static int builtin_cd( parser_t &parser, wchar_t **argv )
 	env_var_t dir_in;
 	wchar_t *dir;
 	int res=STATUS_BUILTIN_OK;
-	void *context = halloc( 0, 0 );
 
 	
 	if( argv[1]  == 0 )
@@ -2615,7 +2614,7 @@ static int builtin_cd( parser_t &parser, wchar_t **argv )
 	else
 		dir_in = argv[1];
 
-	dir = path_get_cdpath( context, dir_in.missing() ? NULL : dir_in.c_str() );
+	dir = path_allocate_cdpath( dir_in.missing() ? NULL : dir_in.c_str() );
 
 	if( !dir )
 	{
@@ -2698,7 +2697,7 @@ static int builtin_cd( parser_t &parser, wchar_t **argv )
 		sb_printf( sb_err, _( L"%ls: Could not set PWD variable\n" ), argv[0] );
 	}
 
-	halloc_free( context );
+    free(dir);
 
 	return res;
 }
diff --git a/common.cpp b/common.cpp
index 3c535621..6d7cf226 100644
--- a/common.cpp
+++ b/common.cpp
@@ -1860,6 +1860,10 @@ void tokenize_variable_array( const wchar_t *val, array_list_t *out )
 	}
 }
 
+bool string_prefixes_string(const wcstring &proposed_prefix, const wcstring &value) {
+    size_t prefix_size = proposed_prefix.size();
+    return prefix_size <= value.size() && value.compare(0, prefix_size, proposed_prefix) == 0;
+}
 
 int create_directory( const wchar_t *d )
 {
diff --git a/common.h b/common.h
index 23eee113..f70815d0 100644
--- a/common.h
+++ b/common.h
@@ -258,6 +258,9 @@ wchar_t *str2wcs_internal( const char *in, wchar_t *out );
 char *wcs2str( const wchar_t *in );
 std::string wcs2string(const wcstring &input);
 
+/** Test if a string prefixes another. Returns true if a is a prefix of b */
+bool string_prefixes_string(const wcstring &proposed_prefix, const wcstring &value);
+
 void assert_is_main_thread(const char *who);
 #define ASSERT_IS_MAIN_THREAD_TRAMPOLINE(x) assert_is_main_thread(x)
 #define ASSERT_IS_MAIN_THREAD() ASSERT_IS_MAIN_THREAD_TRAMPOLINE(__FUNCTION__)
diff --git a/highlight.cpp b/highlight.cpp
index 45632883..7c680475 100644
--- a/highlight.cpp
+++ b/highlight.cpp
@@ -605,16 +605,13 @@ void tokenize( const wchar_t * const buff, int * const color, const int pos, arr
 					
 					if( cmd == L"cd" )
 					{
-                        wcstring dir_str = tok_last( &tok );
-                        if (expand_one(dir_str, EXPAND_SKIP_CMDSUBST))
+                        wcstring dir = tok_last( &tok );
+                        if (expand_one(dir, EXPAND_SKIP_CMDSUBST))
 						{
-                            const wchar_t *dir = dir_str.c_str();
-							int is_long_help = wcsncmp(dir,L"--help", wcslen(dir) );
-							int is_short_help = wcsncmp(dir,L"-h", wcslen(dir) );
-							
-							if( !is_long_help && !is_short_help  && !path_get_cdpath( context, dir ) )
+							int is_help = string_prefixes_string(dir, L"--help") || string_prefixes_string(dir, L"-h");
+							if( !is_help && ! path_can_get_cdpath(dir))
 							{
-								color[ tok_get_pos( &tok ) ] = HIGHLIGHT_ERROR;							
+                                color[ tok_get_pos( &tok ) ] = HIGHLIGHT_ERROR;							
 							}
 						}
 					}
diff --git a/parser.cpp b/parser.cpp
index e92b81f4..59c5f53b 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -1616,7 +1616,7 @@ int parser_t::parse_job( process_t *p,
 					  tokenizer *tok )
 {
 //	array_list_t *args = al_halloc( j );      // The list that will become the argc array for the program
-	std::vector<completion_t> *args = new std::vector<completion_t>();	
+    std::vector<completion_t> args;
 	int use_function = 1;   // May functions be considered when checking what action this command represents
 	int use_builtin = 1;    // May builtins be considered when checking what action this command represents
 	int use_command = 1;    // May commands be considered when checking what action this command represents
@@ -1627,7 +1627,7 @@ int parser_t::parse_job( process_t *p,
 
 	current_tokenizer_pos = tok_get_pos( tok );
 
-	while( args->size() == 0 )
+	while( args.size() == 0 )
 	{
 		wchar_t *nxt=0;
 		int consumed = 0; // Set to one if the command requires a second command, like e.g. while does
@@ -1876,7 +1876,7 @@ int parser_t::parse_job( process_t *p,
 		}
 		completion_t data_to_push;
 		data_to_push.completion = nxt;
-		args->push_back( data_to_push );
+		args.push_back( data_to_push );
 	}
 
 	if( error_code == 0 )
@@ -1884,10 +1884,10 @@ int parser_t::parse_job( process_t *p,
 		if( !p->type )
 		{
 			if( use_builtin &&
-				builtin_exists(args->at(0).completion.c_str()))
+				builtin_exists(args.at(0).completion.c_str()))
 			{
 				p->type = INTERNAL_BUILTIN;
-				is_new_block |= parser_keywords_is_block( args->at( 0 ).completion );
+				is_new_block |= parser_keywords_is_block( args.at( 0 ).completion );
 			}
 		}
 		
@@ -1904,7 +1904,7 @@ int parser_t::parse_job( process_t *p,
 			else
 			{
 				int err;
-				p->actual_cmd = path_get_path( args->at(0).completion.c_str() );
+				p->actual_cmd = path_get_path( args.at(0).completion.c_str() );
 				err = errno;
 				
 				/*
@@ -1918,24 +1918,18 @@ int parser_t::parse_job( process_t *p,
 					  directory, in which case, we use 'cd' as the
 					  implicit command.
 					*/
-					wchar_t *pp =
-						path_get_cdpath( j, args->at(0).completion.c_str() );
-					if( pp )
+					if(path_can_get_cdpath(args.at(0).completion))
 					{
-						wchar_t *tmp;
-
-						tmp = (wchar_t *)wcsdup(args->at( 0 ).completion.c_str());
+						wcstring tmp = args.at(0).completion;
 //						al_truncate( args, 0 );
-						args->clear();
+						args.clear();
 //						al_push( args, halloc_wcsdup( j, L"cd" ) );
 						completion_t comp;
 						comp.completion = L"cd";
-						args->push_back(comp);
+						args.push_back(comp);
 						completion_t comp2;
 						comp2.completion = tmp;
-						args->push_back( comp2 );
-
-//						free(tmp);
+						args.push_back( comp2 );
 						/*
 						  If we have defined a wrapper around cd, use it,
 						  otherwise use the cd builtin
@@ -1948,7 +1942,7 @@ int parser_t::parse_job( process_t *p,
 					else
 					{
 						int tmp;
-						wchar_t *cmd = (wchar_t *)args->at( 0 ).completion.c_str();
+						const wchar_t *cmd = args.at( 0 ).completion.c_str();
 						
 						/* 
 						   We couldn't find the specified command.
@@ -2029,7 +2023,7 @@ int parser_t::parse_job( process_t *p,
 						current_tokenizer_pos=tmp;
 
 						job_set_flag( j, JOB_SKIP, 1 );
-						event_fire_generic(L"fish_command_not_found", (wchar_t *)( args->at( 0 ).completion.c_str() ) );
+						event_fire_generic(L"fish_command_not_found", (wchar_t *)( args.at( 0 ).completion.c_str() ) );
 						proc_set_last_status( err==ENOENT?STATUS_UNKNOWN_COMMAND:STATUS_NOT_EXECUTABLE );
 					}
 				}
@@ -2041,7 +2035,7 @@ int parser_t::parse_job( process_t *p,
 			error( SYNTAX_ERROR,
 				   tok_get_pos( tok ),
 				   UNKNOWN_BUILTIN_ERR_MSG,
-				   args->back().completion.c_str() );
+				   args.back().completion.c_str() );
 		}
 	}
 	
@@ -2118,7 +2112,7 @@ int parser_t::parse_job( process_t *p,
 				p->type = INTERNAL_BLOCK;
 				completion_t data_to_push;
 				data_to_push.completion = sub_block; 
-				args->at( 0 ) =  data_to_push;
+				args.at( 0 ) =  data_to_push;
 			
 				tok_set_pos( tok,
 							 end_pos );
@@ -2137,14 +2131,14 @@ int parser_t::parse_job( process_t *p,
 
 	if( !error_code )
 	{
-		if( p->type == INTERNAL_BUILTIN && parser_keywords_skip_arguments( (wchar_t *)args->at( 0 ).completion.c_str() ) )
+		if( p->type == INTERNAL_BUILTIN && parser_keywords_skip_arguments(args.at(0).completion))
 		{			
 			if( !p->get_argv() )
-                p->set_argv(completions_to_char_arr( *args ));
+                p->set_argv(completions_to_char_arr(args));
 		}
 		else
 		{
-			parse_job_argument_list( p, j, tok, *args );
+			parse_job_argument_list(p, j, tok, args);
 		}
 	}
 
diff --git a/path.cpp b/path.cpp
index be38eaa8..60ce5c5d 100644
--- a/path.cpp
+++ b/path.cpp
@@ -328,7 +328,7 @@ bool path_get_cdpath_string(const wcstring &dir_str, wcstring &result, const env
 	return res;
 }
 
-wchar_t *path_get_cdpath( void *context, const wchar_t *dir )
+wchar_t *path_allocate_cdpath( const wchar_t *dir )
 {
 	wchar_t *res = 0;
 	int err = ENOENT;
@@ -343,7 +343,7 @@ wchar_t *path_get_cdpath( void *context, const wchar_t *dir )
 		{
 			if( S_ISDIR(buf.st_mode) )
 			{
-				res = halloc_wcsdup( context, dir );
+				res = wcsdup(dir);
 			}
 			else
 			{
@@ -399,7 +399,6 @@ wchar_t *path_get_cdpath( void *context, const wchar_t *dir )
 				if( S_ISDIR(buf.st_mode) )
 				{
 					res = whole_path;
-					halloc_register( context, whole_path );					
 					break;
 				}
 				else
@@ -428,6 +427,12 @@ wchar_t *path_get_cdpath( void *context, const wchar_t *dir )
 	return res;
 }
 
+bool path_can_get_cdpath(const wcstring &in) {
+    wchar_t *tmp = path_allocate_cdpath(in.c_str());
+    bool result = (tmp != NULL);
+    free(tmp);
+    return result;
+}
 
 wchar_t *path_get_config( void *context)
 {
diff --git a/path.h b/path.h
index 9994199a..14334af4 100644
--- a/path.h
+++ b/path.h
@@ -49,10 +49,10 @@ bool path_get_path_string(const wcstring &cmd, wcstring &output, const env_vars
    will be returned.
    
    \param in The name of the directory.
-   \param context the halloc context to use for memory allocations
-   \return 0 if the command can not be found, the path of the command otherwise.
+   \return 0 if the command can not be found, the path of the command otherwise. The path should be free'd with free().
 */
-wchar_t *path_get_cdpath( void *context, const wchar_t *in );
+wchar_t *path_allocate_cdpath( const wchar_t *in );
+bool path_can_get_cdpath(const wcstring &in);
 bool path_get_cdpath_string(const wcstring &in, wcstring &out, const env_vars &vars);
 
 /**