From acff2516d41efe572ef607a9fbe73b086a1ad3d1 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 4 Aug 2018 16:15:06 -0700 Subject: [PATCH] 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 | 7 +++---- src/builtin.h | 2 +- src/builtin_argparse.cpp | 2 +- src/builtin_function.cpp | 5 ++--- src/common.h | 16 ++++++++++++++-- src/fish_tests.cpp | 14 ++++++++------ src/proc.h | 4 ++-- 7 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/builtin.cpp b/src/builtin.cpp index 8bf723a68..08e1441c3 100644 --- a/src/builtin.cpp +++ b/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]); diff --git a/src/builtin.h b/src/builtin.h index 9dd03abe8..ef9dc2000 100644 --- a/src/builtin.h +++ b/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 *list); diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp index 538a5ed56..e7c1fa673 100644 --- a/src/builtin_argparse.cpp +++ b/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 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 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, diff --git a/src/builtin_function.cpp b/src/builtin_function.cpp index 7794dc307..7e90c154e 100644 --- a/src/builtin_function.cpp +++ b/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 argv_array(args); - wchar_t **argv = const_cast(argv_array.get()); + null_terminated_array_t argv_array(args); + wchar_t **argv = argv_array.get(); wchar_t *cmd = argv[0]; int argc = builtin_count_args(argv); diff --git a/src/common.h b/src/common.h index 318428661..d9ef38c77 100644 --- a/src/common.h +++ b/src/common.h @@ -523,7 +523,7 @@ char **make_null_terminated_array(const std::vector &lst); // Helper class for managing a null-terminated array of null-terminated strings (of some char type). template 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(); } }; diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 129c6f368..7648d56a2 100644 --- a/src/fish_tests.cpp +++ b/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 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() { diff --git a/src/proc.h b/src/proc.h index 2990f5543..8ed88e26f 100644 --- a/src/proc.h +++ b/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 &get_argv_array(void) const { return argv_array; } + wchar_t **get_argv() { return argv_array.get(); } + const null_terminated_array_t &get_argv_array() const { return argv_array; } /// Returns argv[idx]. const wchar_t *argv(size_t idx) const {