Straighten out some wchar_t** casts

Embrace the fact that builtins expect to modify their argv array and get rid
of a bunch of const.

src/builtin.cpp

@@ -486,7 +486,7 @@ static const wcstring_list_t help_builtins({L"for", L"while", L"function", L"if"
 static bool cmd_needs_help(const wchar_t *cmd) { return contains(help_builtins, cmd); }
 
 /// Execute a builtin command
-int builtin_run(parser_t &parser, const wchar_t *const *argv, io_streams_t &streams) {
+int builtin_run(parser_t &parser, wchar_t **argv, io_streams_t &streams) {
     UNUSED(parser);
     UNUSED(streams);
     if (argv == NULL || argv[0] == NULL) return STATUS_INVALID_ARGS;
@@ -499,15 +499,14 @@ int builtin_run(parser_t &parser, const wchar_t *const *argv, io_streams_t &stre
         return STATUS_CMD_OK;
     }
 
-    const builtin_data_t *data = builtin_lookup(argv[0]);
-    if (data) {
+    if (const builtin_data_t *data = builtin_lookup(argv[0])) {
         // Warning: layering violation and naughty cast. The code originally had a much more
         // complicated solution to achieve exactly the same result: lie about the constness of argv.
         // Some of the builtins we call do mutate the array via their calls to wgetopt() which could
         // result in the pointers being reordered. This is harmless because we only get called once
         // with a given argv array and nothing else will look at the contents of the array after we
         // return.
-        return data->func(parser, streams, (wchar_t **)argv);
+        return data->func(parser, streams, argv);
     }
 
     debug(0, UNKNOWN_BUILTIN_ERR_MSG, argv[0]);

src/builtin.h

@@ -88,7 +88,7 @@ enum { COMMAND_NOT_BUILTIN, BUILTIN_REGULAR, BUILTIN_FUNCTION };
 void builtin_init();
 bool builtin_exists(const wcstring &cmd);
 
-int builtin_run(parser_t &parser, const wchar_t *const *argv, io_streams_t &streams);
+int builtin_run(parser_t &parser, wchar_t **argv, io_streams_t &streams);
 
 wcstring_list_t builtin_get_names();
 void builtin_get_names(std::vector<completion_t> *list);

src/builtin_argparse.cpp

@@ -587,7 +587,7 @@ static int argparse_parse_args(argparse_cmd_opts_t &opts, const wcstring_list_t
     // We need to convert our wcstring_list_t to a <wchar_t **> that can be used by wgetopt_long().
     // This ensures the memory for the data structure is freed when we leave this scope.
     null_terminated_array_t<wchar_t> argv_container(args);
-    auto argv = (wchar_t **)argv_container.get();
+    auto argv = argv_container.get();
 
     int optind;
     int retval = argparse_parse_flags(opts, short_options.c_str(), long_options.data(), cmd, argc,

src/builtin_function.cpp

@@ -212,9 +212,8 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis
     wcstring_list_t args = {L"function"};
     args.insert(args.end(), c_args.begin(), c_args.end());
 
-    // Hackish const_cast matches the one in builtin_run.
-    const null_terminated_array_t<wchar_t> argv_array(args);
-    wchar_t **argv = const_cast<wchar_t **>(argv_array.get());
+    null_terminated_array_t<wchar_t> argv_array(args);
+    wchar_t **argv = argv_array.get();
     wchar_t *cmd = argv[0];
     int argc = builtin_count_args(argv);
 

src/common.h

@@ -523,7 +523,7 @@ char **make_null_terminated_array(const std::vector<std::string> &lst);
 // Helper class for managing a null-terminated array of null-terminated strings (of some char type).
 template <typename CharType_t>
 class null_terminated_array_t {
-    CharType_t **array;
+    CharType_t **array{NULL};
 
     // No assignment or copying.
     void operator=(null_terminated_array_t rhs);
@@ -547,18 +547,30 @@ class null_terminated_array_t {
     }
 
    public:
-    null_terminated_array_t() : array(NULL) {}
+    null_terminated_array_t() = default;
+
     explicit null_terminated_array_t(const string_list_t &argv)
         : array(make_null_terminated_array(argv)) {}
 
     ~null_terminated_array_t() { this->free(); }
 
+    null_terminated_array_t(null_terminated_array_t &&rhs) : array(rhs.array) {
+        rhs.array = nullptr;
+    }
+
+    null_terminated_array_t operator=(null_terminated_array_t &&rhs) {
+        free();
+        array = rhs.array;
+        rhs.array = nullptr;
+    }
+
     void set(const string_list_t &argv) {
         this->free();
         this->array = make_null_terminated_array(argv);
     }
 
     const CharType_t *const *get() const { return array; }
+    CharType_t **get() { return array; }
 
     void clear() { this->free(); }
 };

src/fish_tests.cpp

@@ -2159,14 +2159,16 @@ static void test_test_brackets() {
     parser_t parser;
     io_streams_t streams(0);
 
-    const wchar_t *argv1[] = {L"[", L"foo", NULL};
-    do_test(builtin_test(parser, streams, (wchar_t **)argv1) != 0);
+    null_terminated_array_t<wchar_t> args;
 
-    const wchar_t *argv2[] = {L"[", L"foo", L"]", NULL};
-    do_test(builtin_test(parser, streams, (wchar_t **)argv2) == 0);
+    args.set({L"[", L"foo"});
+    do_test(builtin_test(parser, streams, args.get()) != 0);
 
-    const wchar_t *argv3[] = {L"[", L"foo", L"]", L"bar", NULL};
-    do_test(builtin_test(parser, streams, (wchar_t **)argv3) != 0);
+    args.set({L"[", L"foo", L"]"});
+    do_test(builtin_test(parser, streams, args.get()) == 0);
+
+    args.set({L"[", L"foo", L"]", L"bar"});
+    do_test(builtin_test(parser, streams, args.get()) != 0);
 }
 
 static void test_test() {

src/proc.h

@@ -90,8 +90,8 @@ class process_t {
     void set_argv(const wcstring_list_t &argv) { argv_array.set(argv); }
 
     /// Returns argv.
-    const wchar_t *const *get_argv(void) const { return argv_array.get(); }
-    const null_terminated_array_t<wchar_t> &get_argv_array(void) const { return argv_array; }
+    wchar_t **get_argv() { return argv_array.get(); }
+    const null_terminated_array_t<wchar_t> &get_argv_array() const { return argv_array; }
 
     /// Returns argv[idx].
     const wchar_t *argv(size_t idx) const {