diff --git a/builtin_complete.cpp b/builtin_complete.cpp index 658ff63b6..46c4a9d5a 100644 --- a/builtin_complete.cpp +++ b/builtin_complete.cpp @@ -285,7 +285,7 @@ const wchar_t *builtin_complete_get_temporary_buffer() static int builtin_complete( parser_t &parser, wchar_t **argv ) { ASSERT_IS_MAIN_THREAD(); - int res=0; + bool res=false; int argc=0; int result_mode=SHARED; int remove = 0; @@ -308,7 +308,7 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) woptind=0; - while( res == 0 ) + while( ! res ) { static const struct woption long_options[] = @@ -405,7 +405,7 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) builtin_print_help( parser, argv[0], stderr_buffer ); - res = 1; + res = true; break; case 'x': @@ -436,7 +436,7 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) else { append_format(stderr_buffer, L"%ls: Invalid token '%ls'\n", argv[0], woptarg ); - res = 1; + res = true; } break; } @@ -488,7 +488,7 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) case '?': builtin_unknown_option( parser, argv[0], argv[woptind-1] ); - res = 1; + res = true; break; } @@ -508,7 +508,7 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) parser.test( condition, 0, &stderr_buffer, argv[0] ); - res = 1; + res = true; } } } @@ -526,7 +526,7 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) parser.test_args( comp, &stderr_buffer, argv[0] ); - res = 1; + res = true; } } } @@ -535,8 +535,6 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) { if( do_complete ) { - std::vector comp; - const wchar_t *token; parse_util_token_extent( do_complete_param.c_str(), do_complete_param.size(), &token, 0, 0, 0 ); @@ -547,7 +545,8 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) if( recursion_level < 1 ) { recursion_level++; - + + std::vector comp; complete( do_complete_param, comp, COMPLETE_DEFAULT ); for( size_t i=0; i< comp.size() ; i++ ) @@ -589,7 +588,7 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) argv[0] ); builtin_print_help( parser, argv[0], stderr_buffer ); - res = 1; + res = true; } else if( cmd.empty() && path.empty() ) { @@ -625,5 +624,5 @@ static int builtin_complete( parser_t &parser, wchar_t **argv ) } } - return res; + return res ? 1 : 0; } diff --git a/complete.cpp b/complete.cpp index b203b4a85..21f45ef11 100644 --- a/complete.cpp +++ b/complete.cpp @@ -158,26 +158,35 @@ typedef struct complete_entry_opt typedef std::list option_list_t; class completion_entry_t { - public: - /** Command string */ - wcstring cmd; - - /** True if command is a path */ - bool cmd_is_path; + /** List of all options */ + option_list_t options; /** String containing all short option characters */ wcstring short_opt_str; - /** List of all options */ - option_list_t options; + public: + /** Command string */ + const wcstring cmd; + + /** True if command is a path */ + const bool cmd_is_path; + /** True if no other options than the ones supplied are possible */ bool authoritative; + /** Getters for option list. */ + option_list_t &get_options(); + const option_list_t &get_options() const; + + /** Getter for short_opt_str. */ + wcstring &get_short_opt_str(); + const wcstring &get_short_opt_str() const; + completion_entry_t(const wcstring &c, bool type, const wcstring &options, bool author) : + short_opt_str(options), cmd(c), cmd_is_path(type), - short_opt_str(options), authoritative(author) { } @@ -186,8 +195,32 @@ class completion_entry_t /** Linked list of all completion entries */ typedef std::list completion_entry_list_t; static completion_entry_list_t completion_entries; + +/** The lock that guards the list of completion entries */ static pthread_mutex_t completion_lock = PTHREAD_MUTEX_INITIALIZER; +/** The lock that guards the options list of individual completion entries. If both completion_lock and completion_entry_lock are to be taken, completion_lock must be taken first. */ +static pthread_mutex_t completion_entry_lock = PTHREAD_MUTEX_INITIALIZER; + +option_list_t &completion_entry_t::get_options() { + ASSERT_IS_LOCKED(completion_entry_lock); + return options; +} + +const option_list_t &completion_entry_t::get_options() const { + ASSERT_IS_LOCKED(completion_entry_lock); + return options; +} + +wcstring &completion_entry_t::get_short_opt_str() { + ASSERT_IS_LOCKED(completion_entry_lock); + return short_opt_str; +} + +const wcstring &completion_entry_t::get_short_opt_str() const { + ASSERT_IS_LOCKED(completion_entry_lock); + return short_opt_str; +} /** Class representing an attempt to compute completions */ class completer_t { @@ -360,21 +393,26 @@ void complete_add( const wchar_t *cmd, { CHECK( cmd, ); + /* Lock the lock that allows us to edit the completion entry list */ scoped_lock lock(completion_lock); completion_entry_t *c; c = complete_get_exact_entry( cmd, cmd_is_path ); - c->options.push_front(complete_entry_opt_t()); - complete_entry_opt_t &opt = c->options.front(); + /* Lock the lock that allows us to edit individual completion entries */ + scoped_lock lock2(completion_entry_lock); + + option_list_t &options = c->get_options(); + options.push_front(complete_entry_opt_t()); + complete_entry_opt_t &opt = options.front(); if( short_opt != L'\0' ) { int len = 1 + ((result_mode & NO_COMMON) != 0); - c->short_opt_str.push_back(short_opt); + c->get_short_opt_str().push_back(short_opt); if( len == 2 ) { - c->short_opt_str.push_back(L':'); + c->get_short_opt_str().push_back(L':'); } } @@ -397,13 +435,18 @@ void complete_add( const wchar_t *cmd, static bool complete_remove_entry( completion_entry_t *e, wchar_t short_opt, const wchar_t *long_opt ) { ASSERT_IS_LOCKED(completion_lock); + ASSERT_IS_LOCKED(completion_entry_lock); + option_list_t &options = e->get_options(); if(( short_opt == 0 ) && (long_opt == 0 ) ) { - e->options.clear(); + options.clear(); } else { - for (option_list_t::iterator iter = e->options.begin(); iter != e->options.end(); ) + /* Lock the lock that allows us to edit individual completion entries */ + scoped_lock lock2(completion_entry_lock); + + for (option_list_t::iterator iter = options.begin(); iter != options.end(); ) { complete_entry_opt_t &o = *iter; if(short_opt==o.short_opt || long_opt == o.long_opt) @@ -415,19 +458,20 @@ static bool complete_remove_entry( completion_entry_t *e, wchar_t short_opt, con */ if( o.short_opt ) { - size_t idx = e->short_opt_str.find(o.short_opt); + wcstring &short_opt_str = e->get_short_opt_str(); + size_t idx = short_opt_str.find(o.short_opt); if (idx != wcstring::npos) { /* Consume all colons */ size_t first_non_colon = idx + 1; - while (first_non_colon < e->short_opt_str.size() && e->short_opt_str.at(first_non_colon) == L':') + while (first_non_colon < short_opt_str.size() && short_opt_str.at(first_non_colon) == L':') first_non_colon++; - e->short_opt_str.erase(idx, first_non_colon - idx); + short_opt_str.erase(idx, first_non_colon - idx); } } /* Destroy this option and go to the next one */ - iter = e->options.erase(iter); + iter = options.erase(iter); } else { @@ -436,7 +480,7 @@ static bool complete_remove_entry( completion_entry_t *e, wchar_t short_opt, con } } } - return e->options.empty(); + return options.empty(); } @@ -568,6 +612,7 @@ int complete_is_valid_option( const wchar_t *str, if (allow_autoload) complete_load( cmd, false ); scoped_lock lock(completion_lock); + scoped_lock lock2(completion_entry_lock); for (completion_entry_list_t::iterator iter = completion_entries.begin(); iter != completion_entries.end(); iter++) { const completion_entry_t *i = *iter; @@ -587,11 +632,10 @@ int complete_is_valid_option( const wchar_t *str, break; } - + const option_list_t &options = i->get_options(); if( is_gnu_opt ) { - - for (option_list_t::const_iterator iter = i->options.begin(); iter != i->options.end(); iter++) + for (option_list_t::const_iterator iter = options.begin(); iter != options.end(); iter++) { const complete_entry_opt_t &o = *iter; if( o.old_mode ) @@ -614,7 +658,7 @@ int complete_is_valid_option( const wchar_t *str, else { /* Check for old style options */ - for (option_list_t::const_iterator iter = i->options.begin(); iter != i->options.end(); iter++) + for (option_list_t::const_iterator iter = options.begin(); iter != options.end(); iter++) { const complete_entry_opt_t &o = *iter; @@ -638,12 +682,11 @@ int complete_is_valid_option( const wchar_t *str, for( a = &opt[1]; *a; a++ ) { - //PCA Rewrite this - wchar_t *str_pos = const_cast(wcschr(i->short_opt_str.c_str(), *a)); - - if (str_pos ) + const wcstring &short_opt_str = i->get_short_opt_str(); + size_t str_idx = short_opt_str.find(*a); + if (str_idx != wcstring::npos ) { - if( *(str_pos +1)==L':' ) + if (str_idx + 1 < short_opt_str.size() && short_opt_str.at(str_idx + 1) == L':' ) { /* This is a short option with an embedded argument, @@ -1206,9 +1249,10 @@ bool completer_t::complete_param( const wcstring &scmd_orig, const wcstring &spo complete_load( cmd, true ); scoped_lock lock(completion_lock); + scoped_lock lock2(completion_entry_lock); for (completion_entry_list_t::iterator iter = completion_entries.begin(); iter != completion_entries.end(); iter++) { - completion_entry_t *i = *iter; + const completion_entry_t *i = *iter; const wcstring &match = i->cmd_is_path ? path : cmd; if( ( (!wildcard_match( match, i->cmd ) ) ) ) @@ -1216,6 +1260,7 @@ bool completer_t::complete_param( const wcstring &scmd_orig, const wcstring &spo continue; } + const option_list_t &options = i->get_options(); use_common=1; if( use_switches ) { @@ -1224,7 +1269,7 @@ bool completer_t::complete_param( const wcstring &scmd_orig, const wcstring &spo { /* Check if we are entering a combined option and argument (like --color=auto or -I/usr/include) */ - for (option_list_t::const_iterator oiter = i->options.begin(); oiter != i->options.end(); oiter++) + for (option_list_t::const_iterator oiter = options.begin(); oiter != options.end(); oiter++) { const complete_entry_opt_t *o = &*oiter; wchar_t *arg; @@ -1246,7 +1291,7 @@ bool completer_t::complete_param( const wcstring &scmd_orig, const wcstring &spo If we are using old style long options, check for them first */ - for (option_list_t::const_iterator oiter = i->options.begin(); oiter != i->options.end(); oiter++) + for (option_list_t::const_iterator oiter = options.begin(); oiter != options.end(); oiter++) { const complete_entry_opt_t *o = &*oiter; if( o->old_mode ) @@ -1268,7 +1313,7 @@ bool completer_t::complete_param( const wcstring &scmd_orig, const wcstring &spo */ if( !old_style_match ) { - for (option_list_t::const_iterator oiter = i->options.begin(); oiter != i->options.end(); oiter++) + for (option_list_t::const_iterator oiter = options.begin(); oiter != options.end(); oiter++) { const complete_entry_opt_t *o = &*oiter; /* @@ -1294,7 +1339,7 @@ bool completer_t::complete_param( const wcstring &scmd_orig, const wcstring &spo if( use_common ) { - for (option_list_t::const_iterator oiter = i->options.begin(); oiter != i->options.end(); oiter++) + for (option_list_t::const_iterator oiter = options.begin(); oiter != options.end(); oiter++) { const complete_entry_opt_t *o = &*oiter; /* @@ -1318,7 +1363,7 @@ bool completer_t::complete_param( const wcstring &scmd_orig, const wcstring &spo Check if the short style option matches */ if( o->short_opt != L'\0' && - short_ok( str, o->short_opt, i->short_opt_str ) ) + short_ok( str, o->short_opt, i->get_short_opt_str() ) ) { const wchar_t *desc = o->localized_desc(); wchar_t completion[2]; @@ -1854,10 +1899,12 @@ static void append_switch( wcstring &out, void complete_print( wcstring &out ) { scoped_lock locker(completion_lock); + scoped_lock locker2(completion_entry_lock); for (completion_entry_list_t::const_iterator iter = completion_entries.begin(); iter != completion_entries.end(); iter++) { const completion_entry_t *e = *iter; - for (option_list_t::const_iterator oiter = e->options.begin(); oiter != e->options.end(); oiter++) + const option_list_t options = e->get_options(); + for (option_list_t::const_iterator oiter = options.begin(); oiter != options.end(); oiter++) { const complete_entry_opt_t *o = &*oiter; const wchar_t *modestr[] =