diff --git a/build_tools/lint.fish b/build_tools/lint.fish index 3fd90b244..09ab6f462 100755 --- a/build_tools/lint.fish +++ b/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 diff --git a/src/builtin_string.cpp b/src/builtin_string.cpp index ab864e88d..8911d1ab0 100644 --- a/src/builtin_string.cpp +++ b/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(); } diff --git a/src/common.cpp b/src/common.cpp index 84c3de65a..545288fc0 100644 --- a/src/common.cpp +++ b/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; diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index a3815886a..751136159 100644 --- a/src/env_universal_common.cpp +++ b/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 */ - /* 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. */ - assert(! string_suffixes_string(L"/", directory)); - + // Create and open a temporary file for writing within the given directory. 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. + 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 */ - tmp_name = str2wcstring(narrow_str); - result_fd = wopen_cloexec(tmp_name, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC, 0644); + fcntl(result_fd, F_SETFD, O_CLOEXEC); } #endif - - if (result_fd >= 0) - { - /* Success */ - *out_fd = result_fd; - *out_path = str2wcstring(narrow_str); - success = true; - } + + saved_errno = errno; + success = result_fd != -1; + *out_fd = result_fd; + *out_path = str2wcstring(narrow_str); free(narrow_str); } - if (! success) + + if (!success) { - int err = errno; - report_error(err, L"Unable to open file '%ls'", tmp_name.c_str()); + report_error(saved_errno, L"Unable to open file '%ls'", out_path->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: diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 9afb43441..6250513cb 100644 --- a/src/fish_tests.cpp +++ b/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", - i+1, - str, - (long)token.type); + wprintf(L"Token number %zu of string \n'%ls'\n, got token type %ld\n", + i + 1, str, (long)token.type); } i++; } diff --git a/src/key_reader.cpp b/src/key_reader.cpp index 382f4b229..1cb3b3de4 100644 --- a/src/key_reader.cpp +++ b/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 */ diff --git a/src/parse_tree.cpp b/src/parse_tree.cpp index 5939e270d..51ae77202 100644 --- a/src/parse_tree.cpp +++ b/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; } diff --git a/src/parse_util.cpp b/src/parse_util.cpp index 628e86c59..73db28339 100644 --- a/src/parse_util.cpp +++ b/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: diff --git a/src/wgetopt.cpp b/src/wgetopt.cpp index c2905ab0a..7951eaea0 100644 --- a/src/wgetopt.cpp +++ b/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] == ':')