Lots of miscellaneous cleanup. Unified the path_get_cd_path, path_allocate_cd_path, etc. functions

12 files changed, 91 insertions, 170 deletions

diff --git a/autoload.cpp b/autoload.cpp
index bd9ba326..b1c48ca0 100644
--- a/autoload.cpp
+++ b/autoload.cpp
@@ -116,11 +116,10 @@ int autoload_t::load( const wcstring &cmd, bool reload )
 
 bool autoload_t::can_load( const wcstring &cmd, const env_vars_snapshot_t &vars )
 {
-    const wchar_t *path_var_ptr = vars.get(env_var_name.c_str());
-    if (! path_var_ptr || ! path_var_ptr[0])
+    const env_var_t path_var = vars.get(env_var_name);
+    if (path_var.missing_or_empty())
         return false;
-    
-    const wcstring path_var(path_var_ptr);        
+
     std::vector<wcstring> path_list;
 	tokenize_variable_array( path_var, path_list );
     return this->locate_file_and_maybe_load_it( cmd, false, false, path_list );
diff --git a/builtin.cpp b/builtin.cpp
index 36b8ec7a..401f4c0c 100644
--- a/builtin.cpp
+++ b/builtin.cpp
@@ -2591,7 +2591,7 @@ static int builtin_exit( parser_t &parser, wchar_t **argv )
 static int builtin_cd( parser_t &parser, wchar_t **argv )
 {
 	env_var_t dir_in;
-	wchar_t *dir = NULL;
+	wcstring dir;
 	int res=STATUS_BUILTIN_OK;
 
 	
@@ -2609,11 +2609,13 @@ static int builtin_cd( parser_t &parser, wchar_t **argv )
 		dir_in = argv[1];
     }
 
-    if (! dir_in.missing()) {
-        dir = path_allocate_cdpath(dir_in);
+    bool got_cd_path = false;
+    if (! dir_in.missing())
+    {
+        got_cd_path = path_get_cdpath(dir_in, &dir);
     }
 
-	if( !dir )
+	if( !got_cd_path )
 	{
 		if( errno == ENOTDIR )
 		{
@@ -2665,7 +2667,7 @@ static int builtin_cd( parser_t &parser, wchar_t **argv )
 			append_format(stderr_buffer,
 				   _( L"%ls: Permission denied: '%ls'\n" ),
 				   argv[0],
-				   dir );
+				   dir.c_str() );
 			
 		}
 		else
@@ -2674,7 +2676,7 @@ static int builtin_cd( parser_t &parser, wchar_t **argv )
 			append_format(stderr_buffer,
 				   _( L"%ls: '%ls' is not a directory\n" ),
 				   argv[0],
-				   dir );
+				   dir.c_str() );
 		}
 		
 		if( !get_is_interactive() )
@@ -2690,8 +2692,6 @@ static int builtin_cd( parser_t &parser, wchar_t **argv )
 		append_format(stderr_buffer, _( L"%ls: Could not set PWD variable\n" ), argv[0] );
 	}
 
-    free(dir);
-
 	return res;
 }
 
diff --git a/common.cpp b/common.cpp
index 9b443989..9986c8c1 100644
--- a/common.cpp
+++ b/common.cpp
@@ -219,17 +219,6 @@ int fgetws2( wchar_t **b, int *len, FILE *f )
 
 }
 
-
-static bool string_sort_predicate(const wcstring& d1, const wcstring& d2)
-{
-    return wcsfilecmp(d1.c_str(), d2.c_str()) < 0;
-}
-
-void sort_strings( std::vector<wcstring> &strings)
-{
-    std::sort(strings.begin(), strings.end(), string_sort_predicate);
-}
-
 wchar_t *str2wcs( const char *in )
 {
 	wchar_t *out;
diff --git a/common.h b/common.h
index 4f85d0d6..cdc573c9 100644
--- a/common.h
+++ b/common.h
@@ -207,8 +207,6 @@ int fgetws2( wchar_t **buff, int *len, FILE *f );
 /** Like fgetws2, but reads into a string */
 int fgetws2(wcstring *s, FILE *f);
 
-void sort_strings( std::vector<wcstring> &strings);
-
 /**
    Returns a newly allocated wide character string equivalent of the
    specified multibyte character string
diff --git a/env.cpp b/env.cpp
index 21e68228..fa168c60 100644
--- a/env.cpp
+++ b/env.cpp
@@ -1538,27 +1538,41 @@ void env_export_arr(bool recalc, null_terminated_array_t<char> &output)
 env_vars_snapshot_t::env_vars_snapshot_t(const wchar_t * const *keys)
 {
     ASSERT_IS_MAIN_THREAD();
+    wcstring key;
     for (size_t i=0; keys[i]; i++) {
-        const env_var_t val = env_get_string(keys[i]);
-        if (!val.missing()) {
-            vars[keys[i]] = val;
+        key.assign(keys[i]);
+        const env_var_t val = env_get_string(key);
+        if (! val.missing()) {
+            vars[key] = val;
         }
     }
 }
 
 env_vars_snapshot_t::env_vars_snapshot_t() { }
 
-const wchar_t *env_vars_snapshot_t::get(const wchar_t *key) const
+/* The "current" variables are not a snapshot at all, but instead trampoline to env_get_string, etc. We identify the current snapshot based on pointer values. */
+static const env_vars_snapshot_t sCurrentSnapshot;
+const env_vars_snapshot_t &env_vars_snapshot_t::current()
 {
-    std::map<wcstring, wcstring>::const_iterator iter = vars.find(key);
-    return (iter == vars.end() ? NULL : iter->second.c_str());
+    return sCurrentSnapshot;
 }
 
-env_var_t env_vars_snapshot_t::get(const wcstring &key) const
+bool env_vars_snapshot_t::is_current() const
 {
-    std::map<wcstring, wcstring>::const_iterator iter = vars.find(key);
-    return (iter == vars.end() ? env_var_t::missing_var() : env_var_t(iter->second));
+    return this == &sCurrentSnapshot;
 }
 
-const wchar_t * const env_vars_snapshot_t::highlighting_keys[] = {L"PATH", L"CDPATH", L"HIGHLIGHT_DELAY", L"fish_function_path", NULL};
+env_var_t env_vars_snapshot_t::get(const wcstring &key) const
+{
+    /* If we represent the current state, bounce to env_get_string */
+    if (this->is_current())
+    {
+        return env_get_string(key);
+    }
+    else {
+        std::map<wcstring, wcstring>::const_iterator iter = vars.find(key);
+        return (iter == vars.end() ? env_var_t::missing_var() : env_var_t(iter->second));
+    }
+}
 
+const wchar_t * const env_vars_snapshot_t::highlighting_keys[] = {L"PATH", L"CDPATH", L"fish_function_path", NULL};
diff --git a/env.h b/env.h
index 73f5b984..bd597f1b 100644
--- a/env.h
+++ b/env.h
@@ -183,14 +183,17 @@ int env_set_pwd();
 
 class env_vars_snapshot_t {
     std::map<wcstring, wcstring> vars;
+    bool is_current() const;
 
 public:
     env_vars_snapshot_t(const wchar_t * const * keys);
     env_vars_snapshot_t(void);
     
-    const wchar_t *get(const wchar_t *key) const;
     env_var_t get(const wcstring &key) const;
     
+    // Returns the fake snapshot representing the live variables array
+    static const env_vars_snapshot_t &current();
+    
     // vars necessary for highlighting
     static const wchar_t * const highlighting_keys[];
 };
diff --git a/highlight.cpp b/highlight.cpp
index 57068b1a..a58b1664 100644
--- a/highlight.cpp
+++ b/highlight.cpp
@@ -850,8 +850,9 @@ bool autosuggest_validate_from_history(const history_item_t &item, file_detectio
             if (is_help) {
                 suggestionOK = false;
             } else {
-                wchar_t *path = path_allocate_cdpath(dir, working_directory.c_str());
-                if (path == NULL) {
+                wcstring path;
+                bool can_cd = path_get_cdpath(dir, &path, working_directory.c_str(), vars);
+                if (! can_cd) {
                     suggestionOK = false;
                 } else if (paths_are_same_file(working_directory, path)) {
                     /* Don't suggest the working directory as the path! */
@@ -859,7 +860,6 @@ bool autosuggest_validate_from_history(const history_item_t &item, file_detectio
                 } else {
                     suggestionOK = true;
                 }
-                free(path);
             }
         }        
     } 
@@ -1084,7 +1084,7 @@ static void tokenize( const wchar_t * const buff, std::vector<int> &color, const
                             if (! is_cmd)
                             {
                                 if (use_builtin || (use_function && function_exists_no_autoload( L"cd", vars)))
-                                    is_cmd = path_can_be_implicit_cd(cmd, NULL, working_directory.c_str());
+                                    is_cmd = path_can_be_implicit_cd(cmd, NULL, working_directory.c_str(), vars);
                             }
                             
 							if( is_cmd )
diff --git a/path.cpp b/path.cpp
index 10dbf9be..2ba524ef 100644
--- a/path.cpp
+++ b/path.cpp
@@ -133,7 +133,7 @@ static bool path_get_path_core(const wcstring &cmd, wcstring *out_path, const en
 
 bool path_get_path(const wcstring &cmd, wcstring *out_path, const env_vars_snapshot_t &vars)
 {
-    return path_get_path_core(cmd, out_path, vars.get(wcstring(L"PATH")));
+    return path_get_path_core(cmd, out_path, vars.get(L"PATH"));
 }
 
 bool path_get_path(const wcstring &cmd, wcstring *out_path)
@@ -141,7 +141,7 @@ bool path_get_path(const wcstring &cmd, wcstring *out_path)
     return path_get_path_core(cmd, out_path, env_get_string(L"PATH"));
 }
 
-bool path_get_cdpath_string(const wcstring &dir_str, wcstring &result, const env_vars_snapshot_t &vars)
+bool path_get_cdpath_string(const wcstring &dir_str, wcstring &result, const env_var_t &cdpath)
 {
 	wchar_t *res = 0;
 	int err = ENOENT;
@@ -168,13 +168,12 @@ bool path_get_cdpath_string(const wcstring &dir_str, wcstring &result, const env
 	else
 	{
         
-        const wchar_t *path = L".";
+        wcstring path = L".";
                 
         // Respect CDPATH
         env_var_t cdpath = env_get_string(L"CDPATH");
         if (! cdpath.missing_or_empty()) {
             path = cdpath.c_str();
-            printf("CDPATH: %ls\n", path);
         }
         
         wcstokenizer tokenizer(path, ARRAY_SEP_STR);
@@ -220,21 +219,19 @@ bool path_get_cdpath_string(const wcstring &dir_str, wcstring &result, const env
 	return res;
 }
 
-
-wchar_t *path_allocate_cdpath( const wcstring &dir, const wchar_t *wd )
+bool path_get_cdpath(const wcstring &dir, wcstring *out, const wchar_t *wd, const env_vars_snapshot_t &env_vars)
 {
-	wchar_t *res = NULL;
 	int err = ENOENT;
 	if (dir.empty())
-		return NULL;
+		return false;
         
-    if (wd) {
+    if (wd)
+    {
         size_t len = wcslen(wd);
         assert(wd[len - 1] == L'/');
     }
     
     wcstring_list_t paths;
-    
     if (dir.at(0) == L'/') {
         /* Absolute path */
         paths.push_back(dir);
@@ -248,41 +245,35 @@ wchar_t *path_allocate_cdpath( const wcstring &dir, const wchar_t *wd )
         path.append(dir);
         paths.push_back(path);
     } else {
-        wchar_t *path_cpy;
-        wchar_t *state;
-
         // Respect CDPATH
-        env_var_t path = env_get_string(L"CDPATH");
-        if (path.missing_or_empty()) path = L"."; //We'll change this to the wd if we have one
-
-        path_cpy = wcsdup( path.c_str() );
+        env_var_t path = env_vars.get(L"CDPATH");
+        if (path.missing_or_empty())
+            path = L"."; //We'll change this to the wd if we have one
 
-        for( const wchar_t *nxt_path = wcstok( path_cpy, ARRAY_SEP_STR, &state );
-             nxt_path != NULL;
-             nxt_path = wcstok( 0, ARRAY_SEP_STR, &state) )
+        wcstring nxt_path;
+        wcstokenizer tokenizer(path, ARRAY_SEP_STR);
+        while (tokenizer.next(nxt_path))
         {
         
-            if (! wcscmp(nxt_path, L".") && wd != NULL) {
+            if (nxt_path == L"." && wd != NULL) {
                 // nxt_path is just '.', and we have a working directory, so use the wd instead
                 // TODO: if nxt_path starts with ./ we need to replace the . with the wd
                 nxt_path = wd;
             }
-            
-            wcstring expanded_path = nxt_path;
-            expand_tilde(expanded_path);
+            expand_tilde(nxt_path);
 
 //			debug( 2, L"woot %ls\n", expanded_path.c_str() );
 
-            if (expanded_path.empty())
+            if (nxt_path.empty())
                 continue;
             
-            wcstring whole_path = expanded_path;
+            wcstring whole_path = nxt_path;
             append_path_component(whole_path, dir);
             paths.push_back(whole_path);
         }
-        free( path_cpy );
     }
     
+    bool success = false;
     for (wcstring_list_t::const_iterator iter = paths.begin(); iter != paths.end(); ++iter) {
 		struct stat buf;
         const wcstring &dir = *iter;
@@ -290,7 +281,9 @@ wchar_t *path_allocate_cdpath( const wcstring &dir, const wchar_t *wd )
 		{
 			if( S_ISDIR(buf.st_mode) )
 			{
-				res = wcsdup(dir.c_str());
+                success = true;
+                if (out)
+                    out->assign(dir);
                 break;
 			}
 			else
@@ -300,24 +293,12 @@ wchar_t *path_allocate_cdpath( const wcstring &dir, const wchar_t *wd )
 		}
     }
      
-    if( !res )
-	{
+    if (! success)
 		errno = err;
-	}
-
-	return res;
+    return success;
 }
 
-
-bool path_can_get_cdpath(const wcstring &in, const wchar_t *wd)
-{
-    wchar_t *tmp = path_allocate_cdpath(in, wd);
-    bool result = (tmp != NULL);
-    free(tmp);
-    return result;
-}
-
-bool path_can_be_implicit_cd(const wcstring &path, wcstring *out_path, const wchar_t *wd)
+bool path_can_be_implicit_cd(const wcstring &path, wcstring *out_path, const wchar_t *wd, const env_vars_snapshot_t &vars)
 {
     wcstring exp_path = path;
     expand_tilde(exp_path);
@@ -328,16 +309,8 @@ bool path_can_be_implicit_cd(const wcstring &path, wcstring *out_path, const wch
         string_prefixes_string(L"../", exp_path) ||
         exp_path == L"..")
     {
-        /* These paths can be implicit cd. Note that a single period cannot (that's used for sourcing files anyways) */
-        wchar_t *cd_path = path_allocate_cdpath(exp_path, wd);
-        if (cd_path)
-        {
-            /* It worked. Return the path if desired */
-            if (out_path)
-                out_path->assign(cd_path);
-            free(cd_path);
-            result = true;
-        }
+        /* These paths can be implicit cd, so see if you cd to the path. Note that a single period cannot (that's used for sourcing files anyways) */
+        result = path_get_cdpath(exp_path, out_path, wd, vars);
     }
     return result;
 }
@@ -426,10 +399,10 @@ bool path_is_valid(const wcstring &path, const wcstring &working_directory)
         /* Prepend the working directory. Note that we know path is not empty here. */
         wcstring tmp = working_directory;
         tmp.append(path);
-        path_is_valid = (0 == waccess(tmp.c_str(), F_OK));
+        path_is_valid = (0 == waccess(tmp, F_OK));
     } else {
         /* Simple check */
-        path_is_valid = (0 == waccess(path.c_str(), F_OK));
+        path_is_valid = (0 == waccess(path, F_OK));
     }
     return path_is_valid;
 }
diff --git a/path.h b/path.h
index 2432dafc..2d3cf83d 100644
--- a/path.h
+++ b/path.h
@@ -9,6 +9,8 @@
 #ifndef FISH_PATH_H
 #define FISH_PATH_H
 
+#include "env.h"
+
 /**
    Return value for path_cdpath_get when locatied a rotten symlink
  */
@@ -24,22 +26,16 @@
 bool path_get_config(wcstring &path);
 
 /**
-   Finds the full path of an executable in a newly allocated string.
+   Finds the full path of an executable. Returns YES if successful.
    
    \param cmd The name of the executable.
+   \param output_or_NULL If non-NULL, store the full path.
+   \param vars The environment variables snapshot to use
    \return 0 if the command can not be found, the path of the command otherwise. The result should be freed with free().
 */
-bool path_get_path( const wcstring &cmd, wcstring *output_or_NULL );
-
-/**
-  A version of path_get_path() that takes the user's PATH variable from the given environment variable snapshot
-*/
-class env_vars_snapshot_t;
-bool path_get_path(const wcstring &cmd, wcstring *output_or_NULL, const env_vars_snapshot_t &vars);
-
-
-/** Returns whether the path can be used for an implicit cd command; if so, also returns the path by reference (if desired). This requires it to start with one of the allowed prefixes (., .., ~) and resolve to a directory. */
-bool path_can_be_implicit_cd(const wcstring &path, wcstring *out_path = NULL, const wchar_t *wd = NULL);
+bool path_get_path(const wcstring &cmd,
+                   wcstring *output_or_NULL,
+                   const env_vars_snapshot_t &vars = env_vars_snapshot_t::current());
 
 /**
    Returns the full path of the specified directory, using the CDPATH
@@ -54,14 +50,22 @@ bool path_can_be_implicit_cd(const wcstring &path, wcstring *out_path = NULL, co
    symlink and a file are found, it is undefined which error status
    will be returned.
    
-   \param in The name of the directory.
+   \param dir The name of the directory.
+   \param out_or_NULL If non-NULL, return the path to the resolved directory
    \param wd The working directory, or NULL to use the default. The working directory should have a slash appended at the end.
+   \param vars The environment variable snapshot to use (for the CDPATH variable)
    \return 0 if the command can not be found, the path of the command otherwise. The path should be free'd with free().
 */
+bool path_get_cdpath(const wcstring &dir,
+                     wcstring *out_or_NULL,
+                     const wchar_t *wd = NULL,
+                     const env_vars_snapshot_t &vars = env_vars_snapshot_t::current());
 
-wchar_t *path_allocate_cdpath( const wcstring &in, const wchar_t *wd = NULL);
-bool path_can_get_cdpath(const wcstring &in, const wchar_t *wd = NULL);
-bool path_get_cdpath_string(const wcstring &in, wcstring &out, const env_vars_snapshot_t &vars);
+/** Returns whether the path can be used for an implicit cd command; if so, also returns the path by reference (if desired). This requires it to start with one of the allowed prefixes (., .., ~) and resolve to a directory. */
+bool path_can_be_implicit_cd(const wcstring &path,
+                             wcstring *out_path = NULL,
+                             const wchar_t *wd = NULL,
+                             const env_vars_snapshot_t &vars = env_vars_snapshot_t::current());
 
 /**
    Remove double slashes and trailing slashes from a path,
diff --git a/reader.cpp b/reader.cpp
index b6fb1d17..c32a65ed 100644
--- a/reader.cpp
+++ b/reader.cpp
@@ -2247,18 +2247,9 @@ public:
             // The gen count has changed, so don't do anything
             return 0;
         }
-        const wchar_t *delayer = vars.get(L"HIGHLIGHT_DELAY");
-        double secDelay = 0;
-        if (delayer) {
-            wcstring tmp = delayer;
-            secDelay = from_string<double>(tmp);
-        }
-        if (secDelay > 0) usleep((useconds_t)(secDelay * 1E6));
-        //write(0, "Start", 5);
         if (! string_to_highlight.empty()) {
             highlight_function( string_to_highlight.c_str(), colors, match_highlight_pos, NULL /* error */, vars);
         }
-        //write(0, "End", 3);
         return 0;
     }
 };
diff --git a/util.cpp b/util.cpp
index 5343f332..ce36bd66 100644
--- a/util.cpp
+++ b/util.cpp
@@ -47,41 +47,6 @@
 */
 #define SB_MAX_SIZE (128*1024*1024)
 
-/**
-   Handle oom condition. Default action is to print a stack trace and
-   exit, but an alternative action can be specified.
- */
-#define oom_handler( p )						\
-	{											\
-		if( oom_handler_internal == util_die_on_oom )	\
-		{												\
-			DIE_MEM();									\
-		}												\
-		oom_handler_internal( p );						\
-	}													\
-		
-
-
-void util_die_on_oom( void * p);
-
-void (*oom_handler_internal)(void *) = &util_die_on_oom;
-
-void (*util_set_oom_handler( void (*h)(void *) ))(void *)
-{
-	void (*old)(void *) = oom_handler_internal;
-	
-	if( h )
-		oom_handler_internal = h;
-	else
-		oom_handler_internal = &util_die_on_oom;
-
-	return old;
-}
-
-void util_die_on_oom( void *p )
-{
-}
-
 int mini( int a,
 		  int b )
 {
diff --git a/util.h b/util.h
index 5326602f..4354d52d 100644
--- a/util.h
+++ b/util.h
@@ -27,21 +27,6 @@ typedef struct buffer
 buffer_t;
 
 /**
-   Set the out-of-memory handler callback function. If a memory
-   allocation fails, this function will be called. 
-*/	
-void (*util_set_oom_handler( void (*h)(void *) ))(void *);
-
-/**
-   This is a possible out of memory handler that will kill the current
-   process in response to any out of memory event, while also printing
-   an error message describing what allocation failed.
-
-   This is the default out of memory handler.
-*/
-void util_die_on_oom( void *p );
-
-/**
    Returns the larger of two ints
 */
 int maxi( int a, int b );