fix lint errors that have crept in

src/builtin.cpp

@@ -20,7 +20,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
-#include <stdarg.h>
+#include <signal.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -81,13 +81,6 @@ bool builtin_data_t::operator<(const builtin_data_t *other) const {
     return wcscmp(this->name, other->name) < 0;
 }
 
-static void builtin_append_format(wcstring &str, const wchar_t *fmt, ...) {
-    va_list ap;
-    va_start(ap, fmt);
-    append_formatv(str, fmt, ap);
-    va_end(ap);
-}
-
 /// Counts the number of arguments in the specified null-terminated array
 int builtin_count_args(const wchar_t *const *argv) {
     int argc;
@@ -3051,12 +3044,9 @@ static int disown_job(parser_t &parser, io_streams_t &streams, job_t *j) {
         streams.err.append_format(fmt, L"disown", j->job_id, j->command_wcstr());
     }
 
-    if (parser.job_remove(j)) {
+    if (parser.job_remove(j)) return STATUS_CMD_OK;
-        return STATUS_CMD_OK;
-    } else {
     return STATUS_CMD_ERROR;
 }
-}
 
 /// Builtin for removing jobs from the job list
 static int builtin_disown(parser_t &parser, io_streams_t &streams, wchar_t **argv) {

src/builtin_complete.cpp

@@ -17,7 +17,6 @@
 #include "parse_constants.h"
 #include "parse_util.h"
 #include "parser.h"
-#include "proc.h"
 #include "reader.h"
 #include "wgetopt.h"
 #include "wutil.h"  // IWYU pragma: keep

src/builtin_printf.cpp

@@ -65,7 +65,6 @@
 #include "builtin.h"
 #include "common.h"
 #include "io.h"
-#include "proc.h"
 #include "wutil.h"  // IWYU pragma: keep
 
 class parser_t;

src/builtin_set.cpp

@@ -521,7 +521,6 @@ int builtin_set(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         *wcschr(dest, L'[') = 0;
     }
 
-    wcstring errstr;
     if (!valid_var_name(dest)) {
         streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, dest);
         builtin_print_help(parser, streams, argv[0], streams.err);
@@ -619,7 +618,6 @@ int builtin_set(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
     free(dest);
 
-    if (retcode == STATUS_CMD_OK && preserve_failure_exit_status)
+    if (retcode == STATUS_CMD_OK && preserve_failure_exit_status) retcode = incoming_exit_status;
-        retcode = incoming_exit_status;
     return retcode;
 }

src/builtin_set_color.cpp

@@ -27,7 +27,6 @@
 #include "env.h"
 #include "io.h"
 #include "output.h"
-#include "proc.h"
 #include "wgetopt.h"
 #include "wutil.h"  // IWYU pragma: keep
 

src/builtin_test.cpp

@@ -17,7 +17,6 @@
 #include "builtin.h"
 #include "common.h"
 #include "io.h"
-#include "proc.h"
 #include "wutil.h"  // IWYU pragma: keep
 
 using std::unique_ptr;
@@ -233,7 +232,7 @@ class combining_expression : public expression {
     const std::vector<token_t> combiners;
 
     combining_expression(token_t tok, range_t where, std::vector<unique_ptr<expression>> exprs,
-                         std::vector<token_t> combs)
+                         const std::vector<token_t> &combs)
         : expression(tok, where), subjects(std::move(exprs)), combiners(std::move(combs)) {
         // We should have one more subject than combiner.
         assert(subjects.size() == combiners.size() + 1);

src/builtin_ulimit.cpp

@@ -9,7 +9,6 @@
 #include "common.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "io.h"
-#include "proc.h"
 #include "util.h"
 #include "wgetopt.h"
 #include "wutil.h"  // IWYU pragma: keep

src/color.cpp

@@ -329,6 +329,7 @@ wcstring rgb_color_t::description() const {
         case type_normal: {
             return L"normal";
         }
-        default: { DIE("unknown color type"); }
+        default: { break; }
     }
+    DIE("unknown color type");
 }

src/common.cpp

@@ -1849,6 +1849,8 @@ void scoped_rwlock::unlock_shared(void) {
     locked_shared = false;
 }
 
+#if 0
+// This is not currently used.
 void scoped_rwlock::upgrade(void) {
     assert(locked_shared);
     ASSERT_IS_NOT_FORKED_CHILD();
@@ -1857,6 +1859,7 @@ void scoped_rwlock::upgrade(void) {
     DIE_ON_FAILURE(pthread_rwlock_wrlock(rwlock_obj));
     locked_shared = true;
 }
+#endif
 
 scoped_rwlock::scoped_rwlock(pthread_rwlock_t &rwlock, bool shared)
     : rwlock_obj(&rwlock), locked(false), locked_shared(false) {

src/exec.cpp

@@ -454,14 +454,12 @@ void exec_job(parser_t &parser, job_t *j) {
     // program to exit before continuing in the pipeline, causing the group leader to exit.
     if (j->get_flag(JOB_CONTROL) && !exec_error) {
         for (const process_ptr_t &p : j->processes) {
-            if (p->type != EXTERNAL) {
+            if (p->type != EXTERNAL && (!p->is_last_in_job || !p->is_first_in_job)) {
-                if (!p->is_last_in_job || !p->is_first_in_job) {
                 needs_keepalive = true;
                 break;
             }
         }
     }
-    }
 
     if (needs_keepalive) {
         // Call fork. No need to wait for threads since our use is confined and simple.

src/expand.cpp

@@ -259,7 +259,7 @@ wcstring process_iterator_t::name_for_pid(pid_t pid) {
     }
 
     args = (char *)malloc(maxarg);
-    if (args == NULL) {
+    if (args == NULL) {  // cppcheck-suppress memleak
         return result;
     }
 

src/fallback.cpp

@@ -107,6 +107,7 @@ int fish_mkstemp_cloexec(char *name_template) {
 /// Fallback implementations of wcsdup and wcscasecmp. On systems where these are not needed (e.g.
 /// building on Linux) these should end up just being stripped, as they are static functions that
 /// are not referenced in this file.
+// cppcheck-suppress unusedFunction
 __attribute__((unused)) static wchar_t *wcsdup_fallback(const wchar_t *in) {
     size_t len = wcslen(in);
     wchar_t *out = (wchar_t *)malloc(sizeof(wchar_t) * (len + 1));
@@ -244,6 +245,8 @@ size_t wcslcpy(wchar_t *dst, const wchar_t *src, size_t siz) {
 }
 #endif
 
+#if 0
+// These are not currently used.
 #ifndef HAVE_LRAND48_R
 int lrand48_r(struct drand48_data *buffer, long int *result) {
     *result = rand_r(&buffer->seed);
@@ -255,6 +258,7 @@ int srand48_r(long int seedval, struct drand48_data *buffer) {
     return 0;
 }
 #endif
+#endif
 
 #ifndef HAVE_FUTIMES
 int futimes(int fd, const struct timeval *times) {

src/fallback.h

@@ -138,6 +138,8 @@ wchar_t *wcsndup(const wchar_t *in, size_t c);
 size_t wcslcpy(wchar_t *dst, const wchar_t *src, size_t siz);
 #endif
 
+#if 0
+// These are not currently used.
 #ifndef HAVE_LRAND48_R
 /// Data structure for the lrand48_r fallback implementation.
 struct drand48_data {
@@ -150,6 +152,7 @@ int lrand48_r(struct drand48_data *buffer, long int *result);
 /// Fallback implementation of srand48_r, the seed function for lrand48_r.
 int srand48_r(long int seedval, struct drand48_data *buffer);
 #endif
+#endif
 
 #ifndef HAVE_FUTIMES
 int futimes(int fd, const struct timeval *times);

src/fish_tests.cpp

@@ -3189,7 +3189,8 @@ void history_tests_t::test_history_speed(void)
         if (stop >= end)
             break;
     }
-    fwprintf(stdout, L"%lu items - %.2f msec per item\n", (unsigned long)count, (stop - start) * 1E6 / count);
+    fwprintf(stdout, L"%lu items - %.2f msec per item\n", (unsigned long)count,
+             (stop - start) * 1E6 / count);
     hist->clear();
 }
 #endif
@@ -3833,9 +3834,7 @@ static void test_string(void) {
         {{L"string", L"join", L"", 0}, STATUS_CMD_ERROR, L""},
         {{L"string", L"join", L"", L"", L"", L"", 0}, STATUS_CMD_OK, L"\n"},
         {{L"string", L"join", L"", L"a", L"b", L"c", 0}, STATUS_CMD_OK, L"abc\n"},
-        {{L"string", L"join", L".", L"fishshell", L"com", 0},
+        {{L"string", L"join", L".", L"fishshell", L"com", 0}, STATUS_CMD_OK, L"fishshell.com\n"},
-         STATUS_CMD_OK,
-         L"fishshell.com\n"},
         {{L"string", L"join", L"/", L"usr", 0}, STATUS_CMD_ERROR, L"usr\n"},
         {{L"string", L"join", L"/", L"usr", L"local", L"bin", 0},
          STATUS_CMD_OK,
@@ -3893,9 +3892,7 @@ static void test_string(void) {
         {{L"string", L"match", L"a??B", L"axxb", 0}, STATUS_CMD_ERROR, L""},
         {{L"string", L"match", L"a*b", L"axxbc", 0}, STATUS_CMD_ERROR, L""},
         {{L"string", L"match", L"*b", L"bbba", 0}, STATUS_CMD_ERROR, L""},
-        {{L"string", L"match", L"0x[0-9a-fA-F][0-9a-fA-F]", L"0xbad", 0},
+        {{L"string", L"match", L"0x[0-9a-fA-F][0-9a-fA-F]", L"0xbad", 0}, STATUS_CMD_ERROR, L""},
-         STATUS_CMD_ERROR,
-         L""},
 
         {{L"string", L"match", L"-a", L"*", L"ab", L"cde", 0}, STATUS_CMD_OK, L"ab\ncde\n"},
         {{L"string", L"match", L"*", L"ab", L"cde", 0}, STATUS_CMD_OK, L"ab\ncde\n"},
@@ -3912,9 +3909,7 @@ static void test_string(void) {
         {{L"string", L"match", L"-r", L"a*b", L"b", 0}, STATUS_CMD_OK, L"b\n"},
         {{L"string", L"match", L"-r", L"a*b", L"aab", 0}, STATUS_CMD_OK, L"aab\n"},
         {{L"string", L"match", L"-r", L"-i", L"a*b", L"Aab", 0}, STATUS_CMD_OK, L"Aab\n"},
-        {{L"string", L"match", L"-r", L"-a", L"a[bc]", L"abadac", 0},
+        {{L"string", L"match", L"-r", L"-a", L"a[bc]", L"abadac", 0}, STATUS_CMD_OK, L"ab\nac\n"},
-         STATUS_CMD_OK,
-         L"ab\nac\n"},
         {{L"string", L"match", L"-r", L"a", L"xaxa", L"axax", 0}, STATUS_CMD_OK, L"a\na\n"},
         {{L"string", L"match", L"-r", L"-a", L"a", L"xaxa", L"axax", 0},
          STATUS_CMD_OK,
@@ -3922,16 +3917,12 @@ static void test_string(void) {
         {{L"string", L"match", L"-r", L"a[bc]", L"abadac", 0}, STATUS_CMD_OK, L"ab\n"},
         {{L"string", L"match", L"-r", L"-q", L"a[bc]", L"abadac", 0}, STATUS_CMD_OK, L""},
         {{L"string", L"match", L"-r", L"-q", L"a[bc]", L"ad", 0}, STATUS_CMD_ERROR, L""},
-        {{L"string", L"match", L"-r", L"(a+)b(c)", L"aabc", 0},
+        {{L"string", L"match", L"-r", L"(a+)b(c)", L"aabc", 0}, STATUS_CMD_OK, L"aabc\naa\nc\n"},
-         STATUS_CMD_OK,
-         L"aabc\naa\nc\n"},
         {{L"string", L"match", L"-r", L"-a", L"(a)b(c)", L"abcabc", 0},
          STATUS_CMD_OK,
          L"abc\na\nc\nabc\na\nc\n"},
         {{L"string", L"match", L"-r", L"(a)b(c)", L"abcabc", 0}, STATUS_CMD_OK, L"abc\na\nc\n"},
-        {{L"string", L"match", L"-r", L"(a|(z))(bc)", L"abc", 0},
+        {{L"string", L"match", L"-r", L"(a|(z))(bc)", L"abc", 0}, STATUS_CMD_OK, L"abc\na\nbc\n"},
-         STATUS_CMD_OK,
-         L"abc\na\nbc\n"},
         {{L"string", L"match", L"-r", L"-n", L"a", L"ada", L"dad", 0},
          STATUS_CMD_OK,
          L"1 1\n2 1\n"},
@@ -3953,13 +3944,9 @@ static void test_string(void) {
         {{L"string", L"match", L"-r", L"*", L"", 0}, STATUS_INVALID_ARGS, L""},
         {{L"string", L"match", L"-r", L"-a", L"a*", L"b", 0}, STATUS_CMD_OK, L"\n\n"},
         {{L"string", L"match", L"-r", L"foo\\Kbar", L"foobar", 0}, STATUS_CMD_OK, L"bar\n"},
-        {{L"string", L"match", L"-r", L"(foo)\\Kbar", L"foobar", 0},
+        {{L"string", L"match", L"-r", L"(foo)\\Kbar", L"foobar", 0}, STATUS_CMD_OK, L"bar\nfoo\n"},
-         STATUS_CMD_OK,
-         L"bar\nfoo\n"},
         {{L"string", L"match", L"-r", L"(?=ab\\K)", L"ab", 0}, STATUS_CMD_OK, L"\n"},
-        {{L"string", L"match", L"-r", L"(?=ab\\K)..(?=cd\\K)", L"abcd", 0},
+        {{L"string", L"match", L"-r", L"(?=ab\\K)..(?=cd\\K)", L"abcd", 0}, STATUS_CMD_OK, L"\n"},
-         STATUS_CMD_OK,
-         L"\n"},
 
         {{L"string", L"replace", 0}, STATUS_INVALID_ARGS, L""},
         {{L"string", L"replace", L"", 0}, STATUS_INVALID_ARGS, L""},
@@ -3981,9 +3968,7 @@ static void test_string(void) {
         {{L"string", L"replace", L"-q", L"x", L">x<", L"x", 0}, STATUS_CMD_OK, L""},
         {{L"string", L"replace", L"-a", L"x", L"", L"xxx", 0}, STATUS_CMD_OK, L"\n"},
         {{L"string", L"replace", L"-a", L"***", L"_", L"*****", 0}, STATUS_CMD_OK, L"_**\n"},
-        {{L"string", L"replace", L"-a", L"***", L"***", L"******", 0},
+        {{L"string", L"replace", L"-a", L"***", L"***", L"******", 0}, STATUS_CMD_OK, L"******\n"},
-         STATUS_CMD_OK,
-         L"******\n"},
         {{L"string", L"replace", L"-a", L"a", L"b", L"xax", L"axa", 0},
          STATUS_CMD_OK,
          L"xbx\nbxb\n"},
@@ -3991,9 +3976,7 @@ static void test_string(void) {
         {{L"string", L"replace", L"-r", 0}, STATUS_INVALID_ARGS, L""},
         {{L"string", L"replace", L"-r", L"", 0}, STATUS_INVALID_ARGS, L""},
         {{L"string", L"replace", L"-r", L"", L"", 0}, STATUS_CMD_ERROR, L""},
-        {{L"string", L"replace", L"-r", L"", L"", L"", 0},
+        {{L"string", L"replace", L"-r", L"", L"", L"", 0}, STATUS_CMD_OK, L"\n"},  // pcre2 behavior
-         STATUS_CMD_OK,
-         L"\n"},  // pcre2 behavior
         {{L"string", L"replace", L"-r", L"", L"", L" ", 0},
          STATUS_CMD_OK,
          L" \n"},  // pcre2 behavior
@@ -4001,9 +3984,7 @@ static void test_string(void) {
         {{L"string", L"replace", L"-r", L"a", L"b", L"a", 0}, STATUS_CMD_OK, L"b\n"},
         {{L"string", L"replace", L"-r", L".", L"x", L"abc", 0}, STATUS_CMD_OK, L"xbc\n"},
         {{L"string", L"replace", L"-r", L".", L"", L"abc", 0}, STATUS_CMD_OK, L"bc\n"},
-        {{L"string", L"replace", L"-r", L"(\\w)(\\w)", L"$2$1", L"ab", 0},
+        {{L"string", L"replace", L"-r", L"(\\w)(\\w)", L"$2$1", L"ab", 0}, STATUS_CMD_OK, L"ba\n"},
-         STATUS_CMD_OK,
-         L"ba\n"},
         {{L"string", L"replace", L"-r", L"(\\w)", L"$1$1", L"ab", 0}, STATUS_CMD_OK, L"aab\n"},
         {{L"string", L"replace", L"-r", L"-a", L".", L"x", L"abc", 0}, STATUS_CMD_OK, L"xxx\n"},
         {{L"string", L"replace", L"-r", L"-a", L"(\\w)", L"$1$1", L"ab", 0},

src/history.h

@@ -89,7 +89,7 @@ class history_item_t {
     time_t timestamp() const { return creation_timestamp; }
 
     const path_list_t &get_required_paths() const { return required_paths; }
-    void set_required_paths(path_list_t paths) { required_paths = paths; }
+    void set_required_paths(const path_list_t &paths) { required_paths = paths; }
 
     bool operator==(const history_item_t &other) const {
         return contents == other.contents && creation_timestamp == other.creation_timestamp &&

src/lru.h

@@ -44,7 +44,7 @@ class lru_cache_t {
         // The value from the client
         CONTENTS value;
 
-        explicit lru_node_t(CONTENTS v) : value(std::move(v)) {}
+        explicit lru_node_t(const CONTENTS &v) : value(std::move(v)) {}
     };
 
     // Max node count. This may be (transiently) exceeded by add_node_without_eviction, which is

src/path.cpp

@@ -51,15 +51,13 @@ static bool path_get_path_core(const wcstring &cmd, wcstring *out_path,
     if (!bin_path_var.missing()) {
         bin_path = bin_path_var;
     } else {
-        // Note that PREFIX is defined in the Makefile and is defined when this module is compiled.
+        // Note that PREFIX is defined in the `Makefile` and is thus defined when this module is
-        // This ensures we always default to "/bin", "/usr/bin" and the bin dir defined for the fish
+        // compiled. This ensures we always default to "/bin", "/usr/bin" and the bin dir defined
-        // programs with no duplicates.
+        // for the fish programs. Possibly with a duplicate dir if PREFIX is empty, "/", "/usr" or
-        if (!wcscmp(PREFIX L"/bin", L"/bin") || !wcscmp(PREFIX L"/bin", L"/usr/bin")) {
+        // "/usr/". If the PREFIX duplicates /bin or /usr/bin that is harmless other than a trivial
-            bin_path = L"/bin" ARRAY_SEP_STR L"/usr/bin";
+        // amount of time testing a path we've already tested.
-        } else {
         bin_path = L"/bin" ARRAY_SEP_STR L"/usr/bin" ARRAY_SEP_STR PREFIX L"/bin";
     }
-    }
 
     std::vector<wcstring> pathsv;
     tokenize_variable_array(bin_path, pathsv);

src/proc.cpp

@@ -241,7 +241,7 @@ void job_t::set_flag(job_flag_t flag, bool set) {
     }
 }
 
-bool job_t::get_flag(job_flag_t flag) const { return !!(this->flags & flag); }
+bool job_t::get_flag(job_flag_t flag) const { return (this->flags & flag) == flag; }
 
 int job_signal(job_t *j, int signal) {
     pid_t my_pgid = getpgrp();

src/screen.h

@@ -12,6 +12,7 @@
 
 #include <stddef.h>
 #include <sys/stat.h>
+#include <wchar.h>
 
 #include <algorithm>
 #include <cstddef>

src/wutil.cpp

@@ -458,6 +458,7 @@ int fish_iswalnum(wint_t wc) {
     return iswalnum(wc);
 }
 
+#if 0
 /// We need this because there are too many implementations that don't return the proper answer for
 /// some code points. See issue #3050.
 int fish_iswalpha(wint_t wc) {
@@ -465,6 +466,7 @@ int fish_iswalpha(wint_t wc) {
     if (fish_is_pua(wc)) return 0;
     return iswalpha(wc);
 }
+#endif
 
 /// We need this because there are too many implementations that don't return the proper answer for
 /// some code points. See issue #3050.

src/wutil.h

@@ -102,12 +102,12 @@ int wrename(const wcstring &oldName, const wcstring &newName);
 // We need this because there are too many implementations that don't return the proper answer for
 // some code points. See issue #3050.
 #ifndef FISH_NO_ISW_WRAPPERS
-#define iswalnum fish_iswalnum
 #define iswalpha fish_iswalpha
+#define iswalnum fish_iswalnum
 #define iswgraph fish_iswgraph
 #endif
-int fish_iswalnum(wint_t wc);
 int fish_iswalpha(wint_t wc);
+int fish_iswalnum(wint_t wc);
 int fish_iswgraph(wint_t wc);
 
 int fish_wcswidth(const wchar_t *str);