Fix for miscellaneous issues identiifed by Coverity Scan

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));
 }

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 */
+        }
     }
 }
 

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);
         }

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<SpawnRequest_t *> 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<SpawnRequest_t *> 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. */
+    scoped_lock locker(s_spawn_queue_lock);
-    if (s_active_thread_count == 0)
-        return;
     
 #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();

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;
+        profile_item->skipped = ! populated_job;
-    }
-
-    /* If the job was skipped, we pretend it ran anyways */
-    if (result == parse_execution_skipped)
-    {
-        result = parse_execution_success;
     }
 
 
@@ -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));
 

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());

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;
         }
 

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);

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();

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,);
 

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)

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;