From b0ec7e07b8ee9a58f63638b22c3ff7c2b7891802 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 9 Dec 2022 13:31:19 -0800 Subject: [PATCH] Fix a wgetopt crash and add a test This has apparently been a problem since forever. --- src/fish_tests.cpp | 54 +++++++++++++++++++++++++++++++++++++++++++--- src/wgetopt.cpp | 27 +++++++++++++++++------ 2 files changed, 72 insertions(+), 9 deletions(-) diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index e1c13cdea..f29bca3c7 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -20,6 +20,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -32,9 +35,6 @@ #include #include #include -#include -#include -#include #include #include #include @@ -93,6 +93,7 @@ #include "util.h" #include "wait_handle.h" #include "wcstringutil.h" +#include "wgetopt.h" #include "wildcard.h" #include "wutil.h" // IWYU pragma: keep @@ -7040,6 +7041,51 @@ void termsize_tester_t::test() { do_test(ts2.updating(parser) == *stubby_termsize); } +void test_wgetopt() { + // Regression test for a crash. + const wchar_t *const short_options = L"-a"; + const struct woption long_options[] = { + {L"add", no_argument, 'a'}, + {}, + }; + const wchar_t *argv[] = {L"abbr", L"--add", L"emacsnw", L"emacs", L"-nw", nullptr}; + int argc = builtin_count_args(argv); + wgetopter_t w; + int opt; + int a_count = 0; + wcstring_list_t arguments; + while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, nullptr)) != -1) { + switch (opt) { + case 'a': { + a_count += 1; + break; + } + case 1: { + // non-option argument + do_test(w.woptarg != nullptr); + arguments.push_back(w.woptarg); + break; + } + case '?': { + // unrecognized option + fprintf(stderr, "got arg %d\n", w.woptind - 1); + if (argv[w.woptind - 1]) { + do_test(argv[w.woptind - 1] != nullptr); + arguments.push_back(argv[w.woptind - 1]); + } + break; + } + default: { + err(L"Unexpected option '%d'", opt); + break; + } + } + } + do_test(a_count == 1); + do_test(arguments.size() == 3); + do_test(join_strings(arguments, L' ') == L"emacsnw emacs -nw"); +} + // typedef void (test_entry_point_t)(); using test_entry_point_t = void (*)(); struct test_t { @@ -7159,6 +7205,8 @@ static const test_t s_tests[]{ {TEST_GROUP("re"), test_re_named}, {TEST_GROUP("re"), test_re_name_extraction}, {TEST_GROUP("re"), test_re_substitute}, + {TEST_GROUP("re"), test_re_substitute}, + {TEST_GROUP("wgetopt"), test_wgetopt}, }; void list_tests() { diff --git a/src/wgetopt.cpp b/src/wgetopt.cpp index 5bcd83606..9b4eae9d6 100644 --- a/src/wgetopt.cpp +++ b/src/wgetopt.cpp @@ -405,17 +405,32 @@ int wgetopter_t::_wgetopt_internal(int argc, string_array_t argv, const wchar_t // "u". // // This distinction seems to be the most useful approach. - if (longopts != nullptr && - (argv[woptind][1] == '-' || - (long_only && (argv[woptind][2] || !std::wcschr(shortopts, argv[woptind][1]))))) { - int retval; - if (_handle_long_opt(argc, argv, longopts, longind, long_only, &retval)) return retval; + if (longopts && woptind < argc) { + const wchar_t *arg = argv[woptind]; + assert(arg && "Null arg"); + bool try_long = false; + if (arg[0] == '-' && arg[1] == '-') { + // Like --foo + try_long = true; + } else if (long_only && wcslen(arg) >= 3) { + // Like -fu + try_long = true; + } else if (!std::wcschr(shortopts, arg[1])) { + // Like -f, but f is not a short arg. + try_long = true; + } + if (try_long) { + int retval = 0; + if (_handle_long_opt(argc, argv, longopts, longind, long_only, &retval)) { + return retval; + } + } } - return _handle_short_opt(argc, argv); } int wgetopter_t::wgetopt_long(int argc, string_array_t argv, const wchar_t *options, const struct woption *long_options, int *opt_index) { + assert(woptind <= argc && "woptind is out of range"); return _wgetopt_internal(argc, argv, options, long_options, opt_index, 0); }