diff --git a/builtin.cpp b/builtin.cpp index 74b4ca1f0..9fed9c3e8 100644 --- a/builtin.cpp +++ b/builtin.cpp @@ -3255,7 +3255,7 @@ static int builtin_end( parser_t &parser, wchar_t **argv ) parser.get_job_pos()-parser.current_block->tok_pos ); d->definition = def; - function_add( d, parser ); + function_add( *d, parser ); free( def ); } else diff --git a/builtin_set.cpp b/builtin_set.cpp index c46be8b1a..a29696be1 100644 --- a/builtin_set.cpp +++ b/builtin_set.cpp @@ -92,7 +92,7 @@ static int my_env_set( const wchar_t *key, wcstring_list_t &val, int scope ) { const wchar_t *colon; - append_format(stderr_buffer, _(BUILTIN_SET_PATH_ERROR), L"set", dir, key); + append_format(stderr_buffer, _(BUILTIN_SET_PATH_ERROR), L"set", dir, key); colon = wcschr( dir, L':' ); if( colon && *(colon+1) ) diff --git a/common.cpp b/common.cpp index 012bf13cb..0bdb2035b 100644 --- a/common.cpp +++ b/common.cpp @@ -726,18 +726,19 @@ void debug_safe(int level, const char *msg, const char *param1, const char *para errno = errno_old; } -void format_int_safe(char buff[128], int val) { +void format_long_safe(char buff[128], long val) { if (val == 0) { strcpy(buff, "0"); } else { /* Generate the string in reverse */ size_t idx = 0; bool negative = (val < 0); - if (negative) - val = -val; + + /* Note that we can't just negate val if it's negative, because it may be the most negative value. We do rely on round-towards-zero division though. */ - while (val > 0) { - buff[idx++] = '0' + (val % 10); + while (val != 0) { + long rem = val % 10; + buff[idx++] = '0' + (rem < 0 ? -rem : rem); val /= 10; } if (negative) @@ -753,6 +754,33 @@ void format_int_safe(char buff[128], int val) { } } +void format_long_safe(wchar_t buff[128], long val) { + if (val == 0) { + wcscpy(buff, L"0"); + } else { + /* Generate the string in reverse */ + size_t idx = 0; + bool negative = (val < 0); + + while (val > 0) { + long rem = val % 10; + /* Here we're assuming that wide character digits are contiguous - is that a correct assumption? */ + buff[idx++] = L'0' + (rem < 0 ? -rem : rem); + val /= 10; + } + if (negative) + buff[idx++] = L'-'; + buff[idx] = 0; + + size_t left = 0, right = idx - 1; + while (left < right) { + wchar_t tmp = buff[left]; + buff[left++] = buff[right]; + buff[right--] = tmp; + } + } +} + void write_screen( const wcstring &msg, wcstring &buff ) { const wchar_t *start, *pos; diff --git a/common.h b/common.h index a2e61ce30..a746966df 100644 --- a/common.h +++ b/common.h @@ -281,15 +281,6 @@ void assert_is_locked(void *mutex, const char *who); */ char *wcs2str_internal( const wchar_t *in, char *out ); -/** - Converts some type to a wstring. - */ -template -wcstring format_val(T x) { - std::wstringstream stream; - stream << x; - return stream.str(); -} template T from_string(const wcstring &x) { @@ -678,8 +669,31 @@ void format_size_safe(char buff[128], unsigned long long sz); /** Our crappier versions of debug which is guaranteed to not allocate any memory, or do anything other than call write(). This is useful after a call to fork() with threads. */ void debug_safe(int level, const char *msg, const char *param1 = NULL, const char *param2 = NULL, const char *param3 = NULL); -/** Writes out an int safely */ -void format_int_safe(char buff[128], int val); +/** Writes out a long safely */ +void format_long_safe(char buff[128], long val); +void format_long_safe(wchar_t buff[128], long val); + + +/** Converts some type to a wstring. */ +template +inline wcstring format_val(T x) { + std::wstringstream stream; + stream << x; + return stream.str(); +} + +/* wstringstream is a huge memory pig. Let's provide some specializations where we can. */ +template<> +inline wcstring format_val(long x) { + wchar_t buff[128]; + format_long_safe(buff, x); + return wcstring(buff); +} + +template<> +inline wcstring format_val(int x) { + return format_val(static_cast(x)); +} /** diff --git a/event.cpp b/event.cpp index cf6e9e038..0f87bc39b 100644 --- a/event.cpp +++ b/event.cpp @@ -136,7 +136,7 @@ static int event_match( const event_t *classv, const event_t *instance ) Create an identical copy of an event. Use deep copying, i.e. make duplicates of any strings used as well. */ -static event_t *event_copy( event_t *event, int copy_arguments ) +static event_t *event_copy( const event_t *event, int copy_arguments ) { event_t *e = new event_t(*event); @@ -232,7 +232,7 @@ static void show_all_handlers(void) { } #endif -void event_add_handler( event_t *event ) +void event_add_handler( const event_t *event ) { event_t *e; diff --git a/event.h b/event.h index a32c2b3c2..402b3594b 100644 --- a/event.h +++ b/event.h @@ -100,7 +100,7 @@ struct event_t May not be called by a signal handler, since it may allocate new memory. */ -void event_add_handler( event_t *event ); +void event_add_handler( const event_t *event ); /** Remove all events matching the specified criterion. diff --git a/fish_tests.cpp b/fish_tests.cpp index c52c7cacb..1cc986d59 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -171,10 +171,17 @@ static void test_format(void) { for (int j=-129; j <= 129; j++) { char buff1[128], buff2[128]; - format_int_safe(buff1, j); + format_long_safe(buff1, j); sprintf(buff2, "%d", j); assert( ! strcmp(buff1, buff2)); - } + } + + long q = LONG_MIN; + char buff1[128], buff2[128]; + format_long_safe(buff1, q); + sprintf(buff2, "%ld", q); + assert( ! strcmp(buff1, buff2)); + } /** diff --git a/function.cpp b/function.cpp index 45a0dd154..5b80391b8 100644 --- a/function.cpp +++ b/function.cpp @@ -79,7 +79,7 @@ void function_autoload_t::command_removed(const wcstring &cmd) { that the function being defined is autoloaded. There should be a better way to do this... */ -static int is_autoload = 0; +static bool is_autoload = false; /** Make sure that if the specified function is a dynamically loaded @@ -89,7 +89,7 @@ static int load( const wcstring &name ) { ASSERT_IS_MAIN_THREAD(); scoped_lock lock(functions_lock); - int was_autoload = is_autoload; + bool was_autoload = is_autoload; int res; function_map_t::iterator iter = loaded_functions.find(name); if( iter != loaded_functions.end() && !iter->second->is_autoload ) { @@ -97,7 +97,7 @@ static int load( const wcstring &name ) return 0; } - is_autoload = 1; + is_autoload = true; res = function_autoloader.load( name, true ); is_autoload = was_autoload; return res; @@ -155,36 +155,51 @@ void function_init() VOMIT_ON_FAILURE(pthread_mutexattr_destroy(&a)); } - -void function_add( function_data_t *data, const parser_t &parser ) +function_info_t::function_info_t(const function_data_t &data, const wchar_t *filename, int def_offset, bool autoload) : + definition(data.definition), + description(data.description), + definition_file(intern(filename)), + definition_offset(def_offset), + named_arguments(data.named_arguments), + is_autoload(autoload), + shadows(data.shadows) { - CHECK( ! data->name.empty(), ); - CHECK( data->definition, ); - scoped_lock lock(functions_lock); - function_remove( data->name ); - - shared_ptr &info = loaded_functions[data->name]; - if (! info) { - info.reset(new function_info_t); - } - - info->definition_offset = parse_util_lineno( parser.get_buffer(), parser.current_block->tok_pos )-1; - info->definition = data->definition; +} - if (! data->named_arguments.empty()) { - info->named_arguments.insert(info->named_arguments.end(), data->named_arguments.begin(), data->named_arguments.end()); - } +function_info_t::function_info_t(const function_info_t &data, const wchar_t *filename, int def_offset, bool autoload) : + definition(data.definition), + description(data.description), + definition_file(intern(filename)), + definition_offset(def_offset), + named_arguments(data.named_arguments), + is_autoload(autoload), + shadows(data.shadows) +{ +} + +void function_add( const function_data_t &data, const parser_t &parser ) +{ + ASSERT_IS_MAIN_THREAD(); + + CHECK( ! data.name.empty(), ); + CHECK( data.definition, ); + scoped_lock lock(functions_lock); + + /* Remove the old function */ + function_remove( data.name ); + + /* Create a new function */ + const wchar_t *filename = reader_current_filename(); + int def_offset = parse_util_lineno( parser.get_buffer(), parser.current_block->tok_pos )-1; + function_info_t *new_info = new function_info_t(data, filename, def_offset, is_autoload); + + /* Store it in the loaded_functions map */ + loaded_functions[data.name].reset(new_info); - if (! data->description.empty()) - info->description = data->description; - info->definition_file = intern(reader_current_filename()); - info->is_autoload = is_autoload; - info->shadows = data->shadows; - - - for( size_t i=0; i< data->events.size(); i++ ) + /* Add event handlers */ + for( std::vector::const_iterator iter = data.events.begin(); iter != data.events.end(); ++iter ) { - event_add_handler( &data->events.at(i) ); + event_add_handler( &*iter ); } } @@ -273,21 +288,17 @@ void function_set_desc( const wcstring &name, const wcstring &desc ) if (func) func->description = desc; } -int function_copy( const wcstring &name, const wcstring &new_name ) +bool function_copy( const wcstring &name, const wcstring &new_name ) { - int result = 0; + bool result = false; scoped_lock lock(functions_lock); function_map_t::const_iterator iter = loaded_functions.find(name); if (iter != loaded_functions.end()) { shared_ptr &new_info = loaded_functions[new_name]; - new_info.reset(new function_info_t(*iter->second)); - // This new instance of the function shouldn't be tied to the def - // file of the original. - new_info->definition_file = 0; - new_info->is_autoload = 0; - - result = 1; + // This new instance of the function shouldn't be tied to the definition file of the original, so pass NULL filename, etc. + new_info.reset(new function_info_t(*iter->second, NULL, 0, false)); + result = true; } return result; } diff --git a/function.h b/function.h index 3ede41e8b..4bf4003f7 100644 --- a/function.h +++ b/function.h @@ -60,37 +60,32 @@ struct function_data_t class function_info_t { public: - /** Function definition */ - wcstring definition; + /** Constructs relevant information from the function_data */ + function_info_t(const function_data_t &data, const wchar_t *filename, int def_offset, bool autoload); - /** Function description */ + /** Used by function_copy */ + function_info_t(const function_info_t &data, const wchar_t *filename, int def_offset, bool autoload); + + /** Function definition */ + const wcstring definition; + + /** Function description. Only the description may be changed after the function is created. */ wcstring description; - /** - File where this function was defined (intern'd string) - */ - const wchar_t *definition_file; + /** File where this function was defined (intern'd string) */ + const wchar_t * const definition_file; - /** - Line where definition started - */ - int definition_offset; + /** Line where definition started */ + const int definition_offset; - /** - List of all named arguments for this function - */ - wcstring_list_t named_arguments; + /** List of all named arguments for this function */ + const wcstring_list_t named_arguments; - /** - Flag for specifying that this function was automatically loaded - */ - bool is_autoload; + /** Flag for specifying that this function was automatically loaded */ + const bool is_autoload; - /** - Set to non-zero if invoking this function shadows the variables - of the underlying function. - */ - bool shadows; + /** Set to true if invoking this function shadows the variables of the underlying function. */ + const bool shadows; }; @@ -99,11 +94,8 @@ public: */ void function_init(); -/** - Add a function. The parameters values are copied and should be - freed by the caller. -*/ -void function_add( function_data_t *data, const parser_t &parser ); +/** Add a function. */ +void function_add( const function_data_t &data, const parser_t &parser ); /** Remove the function with the specified name. @@ -172,9 +164,9 @@ wcstring_list_t function_get_named_arguments( const wcstring &name ); /** Creates a new function using the same definition as the specified function. - Returns non-zero if copy is successful. + Returns true if copy is successful. */ -int function_copy( const wcstring &name, const wcstring &new_name ); +bool function_copy( const wcstring &name, const wcstring &new_name ); /** diff --git a/highlight.cpp b/highlight.cpp index f32169ede..abb5a73af 100644 --- a/highlight.cpp +++ b/highlight.cpp @@ -1331,7 +1331,7 @@ static void highlight_universal_internal( const wcstring &buffstr, */ if( (buffstr.at(pos) == L'\'') || (buffstr.at(pos) == L'\"') ) { - std::deque lst; + std::vector lst; int level=0; wchar_t prev_q=0; diff --git a/intern.cpp b/intern.cpp index ced8be9f2..224b9ca6d 100644 --- a/intern.cpp +++ b/intern.cpp @@ -11,6 +11,8 @@ #include #include #include +#include +#include #include "fallback.h" #include "util.h" @@ -20,13 +22,24 @@ #include "intern.h" /** Comparison function for intern'd strings */ -static bool string_table_compare(const wchar_t *a, const wchar_t *b) { - return wcscmp(a, b) < 0; -} +class string_table_compare_t { + public: + bool operator()(const wchar_t *a, const wchar_t *b) const { + return wcscmp(a, b) < 0; + } +}; +/* A sorted deque ends up being a little more memory efficient than a std::set for the intern'd string table */ +#define USE_SET 0 +#if USE_SET /** The table of intern'd strings */ -typedef std::set string_table_t; -static string_table_t string_table(string_table_compare); +typedef std::set string_table_t; +#else +/** The table of intern'd strings */ +typedef std::deque string_table_t; +#endif + +static string_table_t string_table; /** The lock to provide thread safety for intern'd strings */ static pthread_mutex_t intern_lock = PTHREAD_MUTEX_INITIALIZER; @@ -39,6 +52,8 @@ static const wchar_t *intern_with_dup( const wchar_t *in, bool dup ) // debug( 0, L"intern %ls", in ); scoped_lock lock(intern_lock); const wchar_t *result; + +#if USE_SET string_table_t::const_iterator iter = string_table.find(in); if (iter != string_table.end()) { result = *iter; @@ -46,6 +61,15 @@ static const wchar_t *intern_with_dup( const wchar_t *in, bool dup ) result = dup ? wcsdup(in) : in; string_table.insert(result); } +#else + string_table_t::iterator iter = std::lower_bound(string_table.begin(), string_table.end(), in, string_table_compare_t()); + if (iter != string_table.end() && wcscmp(*iter, in) == 0) { + result = *iter; + } else { + result = dup ? wcsdup(in) : in; + string_table.insert(iter, result); + } +#endif return result; } diff --git a/parser.h b/parser.h index 8f2f1adf3..016c98409 100644 --- a/parser.h +++ b/parser.h @@ -12,7 +12,6 @@ #include "event.h" #include "function.h" #include -#include #include #define PARSER_TEST_ERROR 1 @@ -34,7 +33,7 @@ struct event_block_t unsigned int typemask; }; -typedef std::deque event_block_list_t; +typedef std::list event_block_list_t; inline bool event_block_list_blocks_type(const event_block_list_t &ebls, int type) { for (event_block_list_t::const_iterator iter = ebls.begin(); iter != ebls.end(); ++iter) { diff --git a/reader.cpp b/reader.cpp index 3c4c92760..c74ebd2da 100644 --- a/reader.cpp +++ b/reader.cpp @@ -324,10 +324,8 @@ static int is_interactive_read; */ static int end_loop = 0; -/** - The stack containing names of files that are being parsed -*/ -static std::stack current_filename; +/** The stack containing names of files that are being parsed */ +static std::stack > current_filename; /**