diff --git a/color.cpp b/color.cpp index e570b1bc8..5b19b900d 100644 --- a/color.cpp +++ b/color.cpp @@ -338,12 +338,12 @@ void rgb_color_t::parse(const wcstring &str) } } -rgb_color_t::rgb_color_t(const wcstring &str) +rgb_color_t::rgb_color_t(const wcstring &str) : type(), flags() { this->parse(str); } -rgb_color_t::rgb_color_t(const std::string &str) +rgb_color_t::rgb_color_t(const std::string &str) : type(), flags() { this->parse(str2wcstring(str)); } diff --git a/common.cpp b/common.cpp index 6767e3517..473f3502d 100644 --- a/common.cpp +++ b/common.cpp @@ -2176,7 +2176,11 @@ void restore_term_foreground_process_group(void) { if (initial_foreground_process_group != -1) { - tcsetpgrp(STDIN_FILENO, initial_foreground_process_group); + /* This is called during shutdown and from a signal handler. We don't bother to complain on failure. */ + if (0 > tcsetpgrp(STDIN_FILENO, initial_foreground_process_group)) + { + /* Ignore failure */ + } } } diff --git a/history.cpp b/history.cpp index a4ad54354..9bed7b05e 100644 --- a/history.cpp +++ b/history.cpp @@ -542,6 +542,7 @@ history_t::history_t(const wcstring &pname) : disable_automatic_save_counter(0), mmap_start(NULL), mmap_length(0), + mmap_type(history_file_type_t(-1)), mmap_file_id(kInvalidFileID), boundary_timestamp(time(NULL)), countdown_to_vacuum(-1), @@ -1443,7 +1444,10 @@ bool history_t::save_internal_via_rewrite() else { wcstring new_name = history_filename(name, wcstring()); - wrename(tmp_name, new_name); + if (0 > wrename(tmp_name, new_name)) + { + debug(2, L"Error when renaming history file"); + } } close(out_fd); } diff --git a/iothread.cpp b/iothread.cpp index 1fcd137f1..920d2aae1 100644 --- a/iothread.cpp +++ b/iothread.cpp @@ -50,7 +50,7 @@ struct MainThreadRequest_t /* Spawn support. Requests are allocated and come in on request_queue. They go out on result_queue, at which point they can be deallocated. s_active_thread_count is also protected by the lock. */ static pthread_mutex_t s_spawn_queue_lock; static std::queue s_request_queue; -static volatile int s_active_thread_count; +static int s_active_thread_count; static pthread_mutex_t s_result_queue_lock; static std::queue s_result_queue; @@ -267,9 +267,7 @@ void iothread_drain_all(void) ASSERT_IS_MAIN_THREAD(); ASSERT_IS_NOT_FORKED_CHILD(); - /* Hackish. Since we are the only thread that can increment s_active_thread_count, we can check for a zero value without locking; the true value may be smaller than we read, but never bigger. */ - if (s_active_thread_count == 0) - return; + scoped_lock locker(s_spawn_queue_lock); #define TIME_DRAIN 0 #if TIME_DRAIN @@ -280,10 +278,12 @@ void iothread_drain_all(void) /* Nasty polling via select(). */ while (s_active_thread_count > 0) { + locker.unlock(); if (iothread_wait_for_pending_completions(1000)) { iothread_service_completion(); } + locker.lock(); } #if TIME_DRAIN double after = timef(); diff --git a/parse_execution.cpp b/parse_execution.cpp index 3f162f1d5..e190ed8a5 100644 --- a/parse_execution.cpp +++ b/parse_execution.cpp @@ -136,9 +136,9 @@ const parse_node_t *parse_execution_context_t::infinite_recursive_statement_in_j /* Ok, this is an undecorated plain statement. Get and expand its command */ wcstring cmd; tree.command_for_plain_statement(plain_statement, src, &cmd); - expand_one(cmd, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES, NULL); - if (cmd == forbidden_function_name) + if (expand_one(cmd, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES, NULL) && + cmd == forbidden_function_name) { /* This is it */ infinite_recursive_statement = &statement; @@ -1286,15 +1286,6 @@ parse_execution_result_t parse_execution_context_t::populate_job_from_job_node(j parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t &job_node, const block_t *associated_block) { - parse_execution_result_t result = parse_execution_success; - - bool log_it = false; - if (log_it) - { - fprintf(stderr, "%s: %ls\n", __FUNCTION__, get_source(job_node).c_str()); - } - - if (should_cancel_execution(associated_block)) { return parse_execution_cancelled; @@ -1329,6 +1320,8 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t /* When we encounter a block construct (e.g. while loop) in the general case, we create a "block process" that has a pointer to its source. This allows us to handle block-level redirections. However, if there are no redirections, then we can just jump into the block directly, which is significantly faster. */ if (job_is_simple_block(job_node)) { + parse_execution_result_t result = parse_execution_success; + const parse_node_t &statement = *get_child(job_node, 0, symbol_statement); const parse_node_t &specific_statement = *get_child(statement, 0); assert(specific_statement_type_is_redirectable_block(specific_statement)); @@ -1360,7 +1353,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t profile_item->parse = 0; profile_item->exec=(int)(exec_time-start_time); profile_item->cmd = profiling_cmd_name_for_redirectable_block(specific_statement, this->tree, this->src); - profile_item->skipped = result != parse_execution_success; + profile_item->skipped = false; } return result; @@ -1439,13 +1432,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t profile_item->parse = (int)(parse_time-start_time); profile_item->exec=(int)(exec_time-parse_time); profile_item->cmd = j ? j->command() : wcstring(); - profile_item->skipped = ! populated_job || result != parse_execution_success; - } - - /* If the job was skipped, we pretend it ran anyways */ - if (result == parse_execution_skipped) - { - result = parse_execution_success; + profile_item->skipped = ! populated_job; } @@ -1453,7 +1440,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t job_reap(0); /* All done */ - return result; + return parse_execution_success; } parse_execution_result_t parse_execution_context_t::run_job_list(const parse_node_t &job_list_node, const block_t *associated_block) @@ -1481,8 +1468,6 @@ parse_execution_result_t parse_execution_context_t::run_job_list(const parse_nod parse_execution_result_t parse_execution_context_t::eval_node_at_offset(node_offset_t offset, const block_t *associated_block, const io_chain_t &io) { - bool log_it = false; - /* Don't ever expect to have an empty tree if this is called */ assert(! tree.empty()); assert(offset < tree.size()); @@ -1492,11 +1477,6 @@ parse_execution_result_t parse_execution_context_t::eval_node_at_offset(node_off const parse_node_t &node = tree.at(offset); - if (log_it) - { - fprintf(stderr, "eval node: %ls\n", get_source(node).c_str()); - } - /* Currently, we only expect to execute the top level job list, or a block node. Assert that. */ assert(node.type == symbol_job_list || specific_statement_type_is_redirectable_block(node)); diff --git a/parse_productions.cpp b/parse_productions.cpp index 3f905b50c..c1b378ecc 100644 --- a/parse_productions.cpp +++ b/parse_productions.cpp @@ -11,7 +11,8 @@ static bool production_is_empty(const production_t production) /* Empty productions are allowed but must be first. Validate that the given production is in the valid range, i.e. it is either not empty or there is a non-empty production after it */ static bool production_is_valid(const production_options_t production_list, production_option_idx_t which) { - if (which < 0 || which >= MAX_PRODUCTIONS) + assert(which >= 0); + if (which >= MAX_PRODUCTIONS) return false; bool nonempty_found = false; @@ -480,7 +481,7 @@ RESOLVE_ONLY(end_command) #define TEST(sym) case (symbol_##sym): production_list = & productions_ ## sym ; resolver = resolve_ ## sym ; break; const production_t *parse_productions::production_for_token(parse_token_type_t node_type, const parse_token_t &input1, const parse_token_t &input2, production_option_idx_t *out_which_production, wcstring *out_error_text) { - bool log_it = false; + const bool log_it = false; if (log_it) { fprintf(stderr, "Resolving production for %ls with input token <%ls>\n", token_type_description(node_type).c_str(), input1.describe().c_str()); diff --git a/path.cpp b/path.cpp index f436b920c..e61ae6516 100644 --- a/path.cpp +++ b/path.cpp @@ -54,8 +54,6 @@ static bool path_get_path_core(const wcstring &cmd, wcstring *out_path, const en } else { - struct stat buff; - wstat(cmd, &buff); return false; } diff --git a/postfork.cpp b/postfork.cpp index 8222862c2..f47c2e1e2 100644 --- a/postfork.cpp +++ b/postfork.cpp @@ -176,6 +176,7 @@ static int handle_child_io(const io_chain_t &io_chain) { debug_safe_int(1, FD_ERROR, io->fd); safe_perror("dup2"); + exec_close(tmp); return -1; } exec_close(tmp); diff --git a/reader.cpp b/reader.cpp index 09afda1ee..5e386dc7f 100644 --- a/reader.cpp +++ b/reader.cpp @@ -2194,7 +2194,7 @@ static void set_command_line_and_position(editable_line_t *el, const wcstring &n reader_repaint_needed(); } -static void reader_replace_current_token(const wchar_t *new_token) +static void reader_replace_current_token(const wcstring &new_token) { const wchar_t *begin, *end; @@ -2212,7 +2212,7 @@ static void reader_replace_current_token(const wchar_t *new_token) wcstring new_buff(buff, begin - buff); new_buff.append(new_token); new_buff.append(end); - new_pos = (begin-buff) + wcslen(new_token); + new_pos = (begin-buff) + new_token.size(); set_command_line_and_position(el, new_buff, new_pos); } @@ -2255,7 +2255,7 @@ static void handle_token_history(int forward, int reset) if (! data) return; - const wchar_t *str=0; + wcstring str; long current_pos; if (reset) @@ -2278,12 +2278,12 @@ static void handle_token_history(int forward, int reset) { data->search_pos--; } - str = data->search_prev.at(data->search_pos).c_str(); + str = data->search_prev.at(data->search_pos); } else { data->search_pos++; - str = data->search_prev.at(data->search_pos).c_str(); + str = data->search_prev.at(data->search_pos); } reader_replace_current_token(str); @@ -2318,7 +2318,7 @@ static void handle_token_history(int forward, int reset) const wcstring &last = data->search_prev.back(); if (data->search_buff != last) { - str = wcsdup(data->search_buff.c_str()); + str = data->search_buff; } else { @@ -2349,7 +2349,7 @@ static void handle_token_history(int forward, int reset) if (find(data->search_prev.begin(), data->search_prev.end(), last_tok) == data->search_prev.end()) { data->token_history_pos = tok_get_pos(&tok); - str = wcsdup(tok_last(&tok)); + str = tok_last(&tok); } } @@ -2365,7 +2365,7 @@ static void handle_token_history(int forward, int reset) } } - if (str) + if (!str.empty()) { reader_replace_current_token(str); reader_super_highlight_me_plenty(); @@ -3536,7 +3536,7 @@ const wchar_t *reader_readline(int nchars) } else { - reader_replace_current_token(data->search_buff.c_str()); + reader_replace_current_token(data->search_buff); } data->search_buff.clear(); reader_super_highlight_me_plenty(); diff --git a/tokenizer.cpp b/tokenizer.cpp index 659084669..c491bd0e3 100644 --- a/tokenizer.cpp +++ b/tokenizer.cpp @@ -94,7 +94,7 @@ int tok_get_error(tokenizer_t *tok) } -tokenizer_t::tokenizer_t(const wchar_t *b, tok_flags_t flags) : buff(NULL), orig_buff(NULL), last_type(TOK_NONE), last_pos(0), has_next(false), accept_unfinished(false), show_comments(false), last_quote(0), error(0), squash_errors(false), cached_lineno_offset(0), cached_lineno_count(0), continue_line_after_comment(false) +tokenizer_t::tokenizer_t(const wchar_t *b, tok_flags_t flags) : buff(NULL), orig_buff(NULL), last_type(TOK_NONE), last_pos(0), has_next(false), accept_unfinished(false), show_comments(false), show_blank_lines(false), last_quote(0), error(0), squash_errors(false), cached_lineno_offset(0), cached_lineno_count(0), continue_line_after_comment(false) { CHECK(b,); diff --git a/wildcard.cpp b/wildcard.cpp index 305e75f46..3b7736bc6 100644 --- a/wildcard.cpp +++ b/wildcard.cpp @@ -488,7 +488,7 @@ static wcstring complete_get_desc_suffix(const wchar_t *suff_orig) static wcstring file_get_desc(const wcstring &filename, int lstat_res, - struct stat lbuf, + const struct stat &lbuf, int stat_res, struct stat buf, int err) diff --git a/wutil.cpp b/wutil.cpp index 41400e98f..2faec2576 100644 --- a/wutil.cpp +++ b/wutil.cpp @@ -223,9 +223,9 @@ static int wopen_internal(const wcstring &pathname, int flags, mode_t mode, bool { ASSERT_IS_NOT_FORKED_CHILD(); cstring tmp = wcs2string(pathname); - /* Prefer to use O_CLOEXEC. It has to both be defined and nonzero */ + /* Prefer to use O_CLOEXEC. It has to both be defined and nonzero. */ #ifdef O_CLOEXEC - if (cloexec && O_CLOEXEC) + if (cloexec && (O_CLOEXEC != 0)) { flags |= O_CLOEXEC; cloexec = false;