fixes for cppcheck lint warnings

Refine the linting behavior.

Fix several of the, mostly trivial, lint errors.

build_tools/lint.fish

@@ -57,8 +57,14 @@ if set -q c_files[1]
         echo ========================================
         echo Running cppcheck
         echo ========================================
+        # The stderr to stdout redirection is because cppcheck, incorrectly
+        # IMHO, writes its diagnostic messages to stderr. Anyone running
+        # this who wants to capture its output will expect those messages to be
+        # written to stdout.
         cppcheck -q --verbose --std=posix --std=c11 --language=c++ \
-            --inline-suppr --enable=$cppchecks $cppcheck_args $c_files
+            --template "[{file}:{line}]: {severity} ({id}): {message}" \
+            --suppress=missingIncludeSystem \
+            --inline-suppr --enable=$cppchecks $cppcheck_args $c_files 2>&1
     end
 
     if type -q oclint
@@ -66,6 +72,10 @@ if set -q c_files[1]
         echo ========================================
         echo Running oclint
         echo ========================================
+        # The stderr to stdout redirection is because oclint, incorrectly
+        # writes its final summary counts of the errors detected to stderr.
+        # Anyone running this who wants to capture its output will expect those
+        # messages to be written to stdout.
         if test (uname -s) = "Darwin"
             if not test -f compile_commands.json
                 xcodebuild > xcodebuild.log
@@ -73,19 +83,19 @@ if set -q c_files[1]
             end
             if test $all = yes
                 oclint-json-compilation-database -e '/pcre2-10.20/' \
-                    -- -enable-global-analysis
+                    -- -enable-global-analysis 2>&1
             else
                 set i_files
                 for f in $c_files
                     set i_files $i_files -i $f
                 end
                 echo oclint-json-compilation-database -e '/pcre2-10.20/' $i_files
-                oclint-json-compilation-database -e '/pcre2-10.20/' $i_files
+                oclint-json-compilation-database -e '/pcre2-10.20/' $i_files 2>&1
             end
         else
             # Presumably we're on Linux or other platform not requiring special
             # handling for oclint to work.
-            oclint $c_files -- $argv
+            oclint $c_files -- $argv 2>&1
         end
     end
 else

src/builtin_string.cpp

@@ -794,7 +794,7 @@ public:
         size_t arglen = wcslen(arg);
         PCRE2_SIZE bufsize = (arglen == 0) ? 16 : 2 * arglen;
         wchar_t *output = (wchar_t *)malloc(sizeof(wchar_t) * bufsize);
-        if (output == 0)
+        if (output == NULL)
         {
             DIE_MEM();
         }
@@ -820,8 +820,9 @@ public:
                 if (bufsize < MAX_REPLACE_SIZE)
                 {
                     bufsize = std::min(2 * bufsize, MAX_REPLACE_SIZE);
+                    // cppcheck-suppress memleakOnRealloc
                     output = (wchar_t *)realloc(output, sizeof(wchar_t) * bufsize);
-                    if (output == 0)
+                    if (output == NULL)
                     {
                         DIE_MEM();
                     }

src/common.cpp

@@ -1111,12 +1111,6 @@ static void escape_string_internal(const wchar_t *orig_in, size_t in_len, wcstri
 
 wcstring escape(const wchar_t *in, escape_flags_t flags)
 {
-    if (!in)
-    {
-        debug(0, L"%s called with null input", __func__);
-        FATAL_EXIT();
-    }
-
     wcstring result;
     escape_string_internal(in, wcslen(in), &result, flags);
     return result;

src/env_universal_common.cpp

@@ -663,47 +663,44 @@ bool env_universal_t::load()
     return success;
 }
 
-bool env_universal_t::open_temporary_file(const wcstring &directory, wcstring *out_path, int *out_fd)
+bool env_universal_t::open_temporary_file(const wcstring &directory, wcstring *out_path,
+        int *out_fd)
 {
-    /* Create and open a temporary file for writing within the given directory */
+    // Create and open a temporary file for writing within the given directory. Try to create a
-    /* Try to create a temporary file, up to 10 times. We don't use mkstemps because we want to open it CLO_EXEC. This should almost always succeed on the first try. */
+    // temporary file, up to 10 times. We don't use mkstemps because we want to open it CLO_EXEC.
-    assert(! string_suffixes_string(L"/", directory));
+    // This should almost always succeed on the first try.
-    
+    assert(!string_suffixes_string(L"/", directory));
+
     bool success = false;
+    int saved_errno;
     const wcstring tmp_name_template = directory + L"/fishd.tmp.XXXXXX";
     wcstring tmp_name;
+
     for (size_t attempt = 0; attempt < 10 && ! success; attempt++)
     {
         int result_fd = -1;
         char *narrow_str = wcs2str(tmp_name_template.c_str());
 #if HAVE_MKOSTEMP
         result_fd = mkostemp(narrow_str, O_CLOEXEC);
-        if (result_fd >= 0)
-        {
-            tmp_name = str2wcstring(narrow_str);
-        }
 #else
-        if (mktemp(narrow_str))
+        // cppcheck-suppress redundantAssignment
+        result_fd = mkstemp(narrow_str);
+        if (result_fd != -1)
         {
-            /* It was successfully templated; try opening it atomically */
+            fcntl(result_fd, F_SETFD, O_CLOEXEC);
-            tmp_name = str2wcstring(narrow_str);
-            result_fd = wopen_cloexec(tmp_name, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC, 0644);
         }
 #endif
-        
+
-        if (result_fd >= 0)
+        saved_errno = errno;
-        {
+        success = result_fd != -1;
-            /* Success */
+        *out_fd = result_fd;
-            *out_fd = result_fd;
+        *out_path = str2wcstring(narrow_str);
-            *out_path = str2wcstring(narrow_str);
-            success = true;
-        }
         free(narrow_str);
     }
-    if (! success)
+
+    if (!success)
     {
-        int err = errno;
+        report_error(saved_errno, L"Unable to open file '%ls'", out_path->c_str());
-        report_error(err, L"Unable to open file '%ls'", tmp_name.c_str());
     }
     return success;
 }
@@ -1228,6 +1225,7 @@ class universal_notifier_shmem_poller_t : public universal_notifier_t
         
         /* Read the current seed */
         this->poll();
+        // cppcheck-suppress memleak // addr not really leaked
     }
     
     public:

src/fish_tests.cpp

@@ -458,10 +458,8 @@ static void test_tok()
             if (types[i] != token.type)
             {
                 err(L"Tokenization error:");
-                wprintf(L"Token number %d of string \n'%ls'\n, got token type %ld\n",
+                wprintf(L"Token number %zu of string \n'%ls'\n, got token type %ld\n",
-                        i+1,
+                        i + 1, str, (long)token.type);
-                        str,
-                        (long)token.type);
             }
             i++;
         }

src/key_reader.cpp

@@ -82,9 +82,9 @@ int main(int argc, char **argv)
             if ((c=input_common_readch(0)) == EOF)
                 break;
             if ((c > 31) && (c != 127))
-                sprintf(scratch, "dec: %d hex: %x char: %c\n", c, c, c);
+                sprintf(scratch, "dec: %u hex: %x char: %c\n", c, c, c);
             else
-                sprintf(scratch, "dec: %d hex: %x\n", c, c);
+                sprintf(scratch, "dec: %u hex: %x\n", c, c);
             writestr(scratch);
         }
         /* reset the terminal to the saved mode */

src/parse_tree.cpp

@@ -690,7 +690,7 @@ void parse_ll_t::dump_stack(void) const
         }
     }
 
-    fprintf(stderr, "Stack dump (%lu elements):\n", symbol_stack.size());
+    fprintf(stderr, "Stack dump (%zu elements):\n", symbol_stack.size());
     for (size_t idx = 0; idx < lines.size(); idx++)
     {
         fprintf(stderr, "    %ls\n", lines.at(idx).c_str());
@@ -1685,9 +1685,8 @@ enum parse_bool_statement_type_t parse_node_tree_t::statement_boolean_type(const
 bool parse_node_tree_t::job_should_be_backgrounded(const parse_node_t &job) const
 {
     assert(job.type == symbol_job);
-    bool result = false;
     const parse_node_t *opt_background = get_child(job, 2, symbol_optional_background);
-    result = opt_background != NULL && opt_background->tag == parse_background;
+    bool result = opt_background != NULL && opt_background->tag == parse_background;
     return result;
 }
 

src/parse_util.cpp

@@ -1020,14 +1020,16 @@ static const wchar_t *error_format_for_character(wchar_t wc)
 
 void parse_util_expand_variable_error(const wcstring &token, size_t global_token_pos, size_t dollar_pos, parse_error_list_t *errors)
 {
-    // Note that dollar_pos is probably VARIABLE_EXPAND or VARIABLE_EXPAND_SINGLE, not a literal dollar sign
+    // Note that dollar_pos is probably VARIABLE_EXPAND or VARIABLE_EXPAND_SINGLE,
+    // not a literal dollar sign.
     assert(errors != NULL);
     assert(dollar_pos < token.size());
     const bool double_quotes = (token.at(dollar_pos) == VARIABLE_EXPAND_SINGLE);
     const size_t start_error_count = errors->size();
     const size_t global_dollar_pos = global_token_pos + dollar_pos;
     const size_t global_after_dollar_pos = global_dollar_pos + 1;
-    wchar_t char_after_dollar = (dollar_pos + 1 >= token.size() ? L'\0' : token.at(dollar_pos + 1));
+    wchar_t char_after_dollar = dollar_pos + 1 >= token.size() ? 0 : token.at(dollar_pos + 1);
+
     switch (char_after_dollar)
     {
         case BRACKET_BEGIN:

src/wgetopt.cpp

@@ -515,7 +515,7 @@ int wgetopter_t::_wgetopt_internal(int argc, wchar_t **argv, const wchar_t *opts
         {
             if (wopterr)
             {
-                fwprintf(stderr, _(L"%ls: Invalid option -- %lc\n"), argv[0], c);
+                fwprintf(stderr, _(L"%ls: Invalid option -- %lc\n"), argv[0], (wint_t)c);
             }
             woptopt = c;
 
@@ -554,7 +554,7 @@ int wgetopter_t::_wgetopt_internal(int argc, wchar_t **argv, const wchar_t *opts
                     {
                         /* 1003.2 specifies the format of this message.  */
                         fwprintf(stderr, _(L"%ls: Option requires an argument -- %lc\n"),
-                                 argv[0], c);
+                                 argv[0], (wint_t)c);
                     }
                     woptopt = c;
                     if (optstring[0] == ':')