diff --git a/src/builtin.cpp b/src/builtin.cpp index a3cca58f6..ed0874aef 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -1506,8 +1506,8 @@ static int builtin_pwd(parser_t &parser, io_streams_t &streams, wchar_t **argv) return STATUS_BUILTIN_OK; } -static int validate_function_name(int argc, const wchar_t * const *argv, wcstring &function_name, - const wchar_t *cmd, wcstring *out_err) { +static int validate_function_name(int argc, const wchar_t *const *argv, wcstring &function_name, + const wchar_t *cmd, wcstring *out_err) { if (argc < 2) { // This is currently impossible but let's be paranoid. append_format(*out_err, _(L"%ls: Expected function name"), cmd); @@ -1614,7 +1614,6 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis case 'j': case 'p': { pid_t pid; - wchar_t *end; event_t e(EVENT_ANY); if ((opt == 'j') && (wcscasecmp(w.woptarg, L"caller") == 0)) { @@ -1640,15 +1639,14 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis append_format(*out_err, _(L"%ls: Cannot find calling job for event handler"), cmd); return STATUS_BUILTIN_ERROR; - } else { - e.type = EVENT_JOB_ID; - e.param1.job_id = job_id; } + e.type = EVENT_JOB_ID; + e.param1.job_id = job_id; } else { - errno = 0; - pid = fish_wcstoi(w.woptarg, &end, 10); - if (errno || !end || *end) { - append_format(*out_err, _(L"%ls: Invalid process id %ls"), cmd, w.woptarg); + pid = fish_wcstoi(w.woptarg); + if (errno || pid < 0) { + append_format(*out_err, _(L"%ls: Invalid process id '%ls'"), cmd, + w.woptarg); return STATUS_BUILTIN_ERROR; } @@ -1703,10 +1701,9 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis for (int i = w.woptind; i < argc; i++) { named_arguments.push_back(argv[i]); } - } - else { + } else { append_format(*out_err, _(L"%ls: Unexpected positional argument '%ls'"), cmd, - argv[w.woptind]); + argv[w.woptind]); return STATUS_BUILTIN_ERROR; } } @@ -1786,12 +1783,8 @@ static int builtin_random(parser_t &parser, io_streams_t &streams, wchar_t **arg lrand48_r(&seed_buffer, &res); streams.out.append_format(L"%ld\n", res % 32768); } else if (arg_count == 1) { - long foo; - wchar_t *end = 0; - - errno = 0; - foo = wcstol(argv[w.woptind], &end, 10); - if (errno || *end) { + long foo = fish_wcstol(argv[w.woptind]); + if (errno) { streams.err.append_format(_(L"%ls: Seed value '%ls' is not a valid number\n"), argv[0], argv[w.woptind]); return STATUS_BUILTIN_ERROR; @@ -1819,7 +1812,6 @@ static int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) int exit_res = STATUS_BUILTIN_OK; const wchar_t *mode_name = READ_MODE_NAME; int nchars = 0; - wchar_t *end; int shell = 0; int array = 0; bool split_null = false; @@ -1891,9 +1883,8 @@ static int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) break; } case L'n': { - errno = 0; - nchars = fish_wcstoi(w.woptarg, &end, 10); - if (errno || *end != 0) { + nchars = fish_wcstoi(w.woptarg); + if (errno) { if (errno == ERANGE) { streams.err.append_format(_(L"%ls: Argument '%ls' is out of range\n"), argv[0], w.woptarg); @@ -2429,10 +2420,8 @@ static int builtin_exit(parser_t &parser, io_streams_t &streams, wchar_t **argv) if (argc == 1) { ec = proc_get_last_status(); } else { - wchar_t *end; - errno = 0; - ec = wcstol(argv[1], &end, 10); - if (errno || *end != 0) { + ec = fish_wcstol(argv[1]); + if (errno) { streams.err.append_format(_(L"%ls: Argument '%ls' must be an integer\n"), argv[0], argv[1]); builtin_print_help(parser, streams, argv[0], streams.err); @@ -2676,13 +2665,11 @@ static int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) { // Specifying more than one job to put to the foreground is a syntax error, we still // try to locate the job argv[1], since we want to know if this is an ambigous job // specification or if this is an malformed job id. - wchar_t *endptr; int pid; int found_job = 0; - errno = 0; - pid = fish_wcstoi(argv[1], &endptr, 10); - if (!(*endptr || errno)) { + pid = fish_wcstoi(argv[1]); + if (!(errno || pid < 0)) { j = job_get_from_pid(pid); if (j) found_job = 1; } @@ -2698,12 +2685,8 @@ static int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) { j = 0; } else { - wchar_t *end; - int pid; - errno = 0; - pid = abs(fish_wcstoi(argv[1], &end, 10)); - - if (*end || errno) { + int pid = abs(fish_wcstoi(argv[1])); + if (errno) { streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, argv[0], argv[1]); builtin_print_help(parser, streams, argv[0], streams.err); } else { @@ -2785,14 +2768,12 @@ static int builtin_bg(parser_t &parser, io_streams_t &streams, wchar_t **argv) { res = send_to_bg(parser, streams, j, _(L"(default)")); } } else { - wchar_t *end; int i; int pid; for (i = 1; argv[i]; i++) { - errno = 0; - pid = fish_wcstoi(argv[i], &end, 10); - if (errno || pid < 0 || *end || !job_get_from_pid(pid)) { + pid = fish_wcstoi(argv[i]); + if (errno || pid < 0 || !job_get_from_pid(pid)) { streams.err.append_format(_(L"%ls: '%ls' is not a job\n"), argv[0], argv[i]); return STATUS_BUILTIN_ERROR; } @@ -2871,10 +2852,8 @@ static int builtin_return(parser_t &parser, io_streams_t &streams, wchar_t **arg int status; if (argc == 2) { - wchar_t *end; - errno = 0; - status = fish_wcstoi(argv[1], &end, 10); - if (errno || *end != 0) { + status = fish_wcstoi(argv[1]); + if (errno) { streams.err.append_format(_(L"%ls: Argument '%ls' must be an integer\n"), argv[0], argv[1]); builtin_print_help(parser, streams, argv[0], streams.err); @@ -3042,9 +3021,8 @@ static int builtin_history(parser_t &parser, io_streams_t &streams, wchar_t **ar break; } case 'n': { - wchar_t *end = 0; - max_items = wcstol(w.woptarg, &end, 10); - if (!(*w.woptarg != L'\0' && *end == L'\0')) { + max_items = fish_wcstol(w.woptarg); + if (errno) { streams.err.append_format(_(L"%ls: max value '%ls' is not a valid number\n"), argv[0], w.woptarg); return STATUS_BUILTIN_ERROR; @@ -3065,9 +3043,8 @@ static int builtin_history(parser_t &parser, io_streams_t &streams, wchar_t **ar } case '?': { // Try to parse it as a number; e.g., "-123". - wchar_t *end = 0; - max_items = wcstol(argv[w.woptind - 1] + 1, &end, 10); - if (!(argv[w.woptind - 1][1] != L'\0' && *end == L'\0')) { + max_items = fish_wcstol(argv[w.woptind - 1] + 1); + if (errno) { streams.err.append_format(BUILTIN_ERR_UNKNOWN, cmd, argv[w.woptind - 1]); return STATUS_BUILTIN_ERROR; } diff --git a/src/builtin_commandline.cpp b/src/builtin_commandline.cpp index 9941a62cc..8dc9d8e9c 100644 --- a/src/builtin_commandline.cpp +++ b/src/builtin_commandline.cpp @@ -426,12 +426,8 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv) if (cursor_mode) { if (argc - w.woptind) { - wchar_t *endptr; - long new_pos; - errno = 0; - - new_pos = wcstol(argv[w.woptind], &endptr, 10); - if (*endptr || errno) { + long new_pos = fish_wcstol(argv[w.woptind]); + if (errno) { streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, argv[0], argv[w.woptind]); builtin_print_help(parser, streams, argv[0], streams.err); } diff --git a/src/builtin_jobs.cpp b/src/builtin_jobs.cpp index 7a92e6b9e..456385672 100644 --- a/src/builtin_jobs.cpp +++ b/src/builtin_jobs.cpp @@ -186,11 +186,8 @@ int builtin_jobs(parser_t &parser, io_streams_t &streams, wchar_t **argv) { int i; for (i = w.woptind; i < argc; i++) { - int pid; - wchar_t *end; - errno = 0; - pid = fish_wcstoi(argv[i], &end, 10); - if (errno || *end) { + int pid = fish_wcstoi(argv[i]); + if (errno || pid < 0) { streams.err.append_format(_(L"%ls: '%ls' is not a job\n"), argv[0], argv[i]); return 1; } diff --git a/src/builtin_set.cpp b/src/builtin_set.cpp index a9dd120d4..b89c12e23 100644 --- a/src/builtin_set.cpp +++ b/src/builtin_set.cpp @@ -192,12 +192,9 @@ static int parse_index(std::vector &indexes, const wchar_t *src, const wch while (iswspace(*src)) src++; while (*src != L']') { - wchar_t *end; - long l_ind; - - errno = 0; - l_ind = wcstol(src, &end, 10); - if (end == src || errno) { + const wchar_t *end; + long l_ind = fish_wcstol(src, &end); + if (errno > 0) { // ignore errno == -1 meaning the int did not end with a '\0' streams.err.append_format(_(L"%ls: Invalid index starting at '%ls'\n"), L"set", src); return 0; } @@ -207,8 +204,8 @@ static int parse_index(std::vector &indexes, const wchar_t *src, const wch src = end; //!OCLINT(parameter reassignment) if (*src == L'.' && *(src + 1) == L'.') { src += 2; - long l_ind2 = wcstol(src, &end, 10); - if (end == src || errno) { + long l_ind2 = fish_wcstol(src, &end); + if (errno > 0) { // ignore errno == -1 meaning the int did not end with a '\0' return 1; } src = end; //!OCLINT(parameter reassignment) @@ -347,8 +344,6 @@ int builtin_set(parser_t &parser, io_streams_t &streams, wchar_t **argv) { int scope; int slice = 0; - const wchar_t *bad_char = NULL; - // Parse options to obtain the requested operation and the modifiers. w.woptind = 0; while (1) { diff --git a/src/builtin_string.cpp b/src/builtin_string.cpp index afa11ee4d..c0c3a5bdc 100644 --- a/src/builtin_string.cpp +++ b/src/builtin_string.cpp @@ -878,10 +878,8 @@ static int string_split(parser_t &parser, io_streams_t &streams, int argc, wchar break; } case 'm': { - errno = 0; - wchar_t *endptr = 0; - max = wcstol(w.woptarg, &endptr, 10); - if (*endptr != L'\0' || errno != 0) { + max = fish_wcstol(w.woptarg); + if (errno) { string_error(streams, BUILTIN_ERR_NOT_NUMBER, argv[0], w.woptarg); return BUILTIN_STRING_ERROR; } @@ -969,7 +967,7 @@ static int string_sub(parser_t &parser, io_streams_t &streams, int argc, wchar_t long length = -1; bool quiet = false; wgetopter_t w; - wchar_t *endptr = NULL; + for (;;) { int c = w.wgetopt_long(argc, argv, short_options, long_options, 0); @@ -981,16 +979,14 @@ static int string_sub(parser_t &parser, io_streams_t &streams, int argc, wchar_t break; } case 'l': { - errno = 0; - length = wcstol(w.woptarg, &endptr, 10); - if (*endptr != L'\0' || (errno != 0 && errno != ERANGE)) { - string_error(streams, BUILTIN_ERR_NOT_NUMBER, argv[0], w.woptarg); - return BUILTIN_STRING_ERROR; - } + length = fish_wcstol(w.woptarg); if (length < 0 || errno == ERANGE) { string_error(streams, _(L"%ls: Invalid length value '%ls'\n"), argv[0], w.woptarg); return BUILTIN_STRING_ERROR; + } else if (errno) { + string_error(streams, BUILTIN_ERR_NOT_NUMBER, argv[0], w.woptarg); + return BUILTIN_STRING_ERROR; } break; } @@ -999,16 +995,14 @@ static int string_sub(parser_t &parser, io_streams_t &streams, int argc, wchar_t break; } case 's': { - errno = 0; - start = wcstol(w.woptarg, &endptr, 10); - if (*endptr != L'\0' || (errno != 0 && errno != ERANGE)) { - string_error(streams, BUILTIN_ERR_NOT_NUMBER, argv[0], w.woptarg); - return BUILTIN_STRING_ERROR; - } + start = fish_wcstol(w.woptarg); if (start == 0 || start == LONG_MIN || errno == ERANGE) { string_error(streams, _(L"%ls: Invalid start value '%ls'\n"), argv[0], w.woptarg); return BUILTIN_STRING_ERROR; + } else if (errno) { + string_error(streams, BUILTIN_ERR_NOT_NUMBER, argv[0], w.woptarg); + return BUILTIN_STRING_ERROR; } break; } diff --git a/src/builtin_ulimit.cpp b/src/builtin_ulimit.cpp index 8e20702e8..85636ba3f 100644 --- a/src/builtin_ulimit.cpp +++ b/src/builtin_ulimit.cpp @@ -4,7 +4,6 @@ #include #include #include -#include #include "builtin.h" #include "common.h" @@ -293,9 +292,8 @@ int builtin_ulimit(parser_t &parser, io_streams_t &streams, wchar_t **argv) { } else if (wcscasecmp(argv[w.woptind], L"soft") == 0) { new_limit = get(what, soft); } else { - wchar_t *end; - new_limit = wcstol(argv[w.woptind], &end, 10); - if (*end) { + new_limit = fish_wcstol(argv[w.woptind]); + if (errno) { streams.err.append_format(_(L"%ls: Invalid limit '%ls'\n"), cmd, argv[w.woptind]); builtin_print_help(parser, streams, cmd, streams.err); return STATUS_BUILTIN_ERROR; diff --git a/src/env.cpp b/src/env.cpp index d95e7e0cc..cf49e7eb1 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -412,10 +411,10 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { const env_var_t shlvl_str = env_get_string(L"SHLVL"); wcstring nshlvl_str = L"1"; if (!shlvl_str.missing()) { - wchar_t *end; - long shlvl_i = wcstol(shlvl_str.c_str(), &end, 10); - while (iswspace(*end)) ++end; // skip trailing whitespace - if (shlvl_i >= 0 && *end == '\0') { + const wchar_t *end; + // TODO: Figure out how to handle invalid numbers better. Shouldn't we issue a diagnostic? + long shlvl_i = fish_wcstol(shlvl_str.c_str(), &end); + if (!errno && shlvl_i >= 0) { nshlvl_str = to_string(shlvl_i + 1); } } @@ -514,14 +513,10 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode) } if (key == L"umask") { - wchar_t *end; - // Set the new umask. if (val && wcslen(val)) { - errno = 0; - long mask = wcstol(val, &end, 8); - - if (!errno && (!*end) && (mask <= 0777) && (mask >= 0)) { + long mask = fish_wcstol(val, NULL, 8); + if (!errno && mask <= 0777 && mask >= 0) { umask(mask); // Do not actually create a umask variable, on env_get, it will be calculated // dynamically. diff --git a/src/exec.cpp b/src/exec.cpp index b98a1d23f..0e26cfb14 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -11,13 +11,11 @@ #include #include #include -#include #include #include #ifdef HAVE_SPAWN_H #include #endif -#include #include #include #include @@ -420,10 +418,8 @@ void exec_job(parser_t &parser, job_t *j) { const env_var_t shlvl_str = env_get_string(L"SHLVL", ENV_GLOBAL | ENV_EXPORT); wcstring nshlvl_str = L"0"; if (!shlvl_str.missing()) { - wchar_t *end; - long shlvl_i = wcstol(shlvl_str.c_str(), &end, 10); - while (iswspace(*end)) ++end; // skip trailing whitespace - if (shlvl_i > 0 && *end == '\0') { + long shlvl_i = fish_wcstol(shlvl_str.c_str()); + if (!errno && shlvl_i > 0) { nshlvl_str = to_string(shlvl_i - 1); } } diff --git a/src/expand.cpp b/src/expand.cpp index 664e2d4e5..0307c1c80 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -393,7 +393,10 @@ bool process_iterator_t::next_process(wcstring *out_str, pid_t *out_pid) { if (buf.st_uid != getuid()) continue; // Remember the pid. - pid = fish_wcstoi(name.c_str(), NULL, 10); + pid = fish_wcstoi(name.c_str()); + if (errno || pid < 0) { + debug(1, _(L"Unexpected failure to convert pid '%ls' to integer\n"), name.c_str()); + } // The 'cmdline' file exists, it should contain the commandline. FILE *cmdfile; @@ -477,12 +480,8 @@ static int find_job(const struct find_job_data_t *info) { } } } else { - int jid; - wchar_t *end; - - errno = 0; - jid = fish_wcstoi(proc, &end, 10); - if (jid > 0 && !errno && !*end) { + int jid = fish_wcstoi(proc); + if (!errno && jid > 0) { j = job_get(jid); if ((j != 0) && (j->command_wcstr() != 0) && (!j->command_is_empty())) { append_completion(&completions, to_string(j->pgid)); @@ -643,25 +642,22 @@ static bool expand_pid(const wcstring &instr_with_sep, expand_flags_t flags, /// with [. static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector &idx, std::vector &source_positions, size_t array_size) { - wchar_t *end; - const long size = (long)array_size; size_t pos = 1; // skip past the opening square bracket - // debug( 0, L"parse_slice on '%ls'", in ); while (1) { - long tmp; - while (iswspace(in[pos]) || (in[pos] == INTERNAL_SEPARATOR)) pos++; if (in[pos] == L']') { pos++; break; } - errno = 0; const size_t i1_src_pos = pos; - tmp = wcstol(&in[pos], &end, 10); - if ((errno) || (end == &in[pos])) { + const wchar_t *end; + long tmp = fish_wcstol(&in[pos], &end); + // We don't test `*end` as is typically done because we expect it to not be the null char. + // Ignore the case of errno==-1 because it means the end char wasn't the null char. + if (errno > 0) { return pos; } // debug( 0, L"Push idx %d", tmp ); @@ -674,8 +670,9 @@ static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector 0) { return pos; } pos = end - in; @@ -698,11 +695,8 @@ static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector // IWYU pragma: no_include #include +#include #include #include #include #include #include #include -#include #include +#include #include #include #include @@ -99,10 +100,10 @@ static bool should_test_function(const char *func_name) { static int err_count = 0; /// Print formatted output. -static void say(const wchar_t *blah, ...) { +static void say(const wchar_t *fmt, ...) { va_list va; - va_start(va, blah); - vwprintf(blah, va); + va_start(va, fmt); + vwprintf(fmt, va); va_end(va); wprintf(L"\n"); } @@ -156,21 +157,100 @@ static int chdir_set_pwd(const char *path) { // The odd formulation of these macros is to avoid "multiple unary operator" warnings from oclint // were we to use the more natural "if (!(e)) err(..." form. We have to do this because the rules // for the C preprocessor make it practically impossible to embed a comment in the body of a macro. -#define do_test(e) \ - do { \ - if (e) { ; } else { err(L"Test failed on line %lu: %s", __LINE__, #e); } \ +#define do_test(e) \ + do { \ + if (e) { \ + ; \ + } else { \ + err(L"Test failed on line %lu: %s", __LINE__, #e); \ + } \ } while (0) -#define do_test_from(e, from) \ - do { \ - if (e) { ; } else { err(L"Test failed on line %lu (from %lu): %s", __LINE__, from, #e); } \ +#define do_test_from(e, from) \ + do { \ + if (e) { \ + ; \ + } else { \ + err(L"Test failed on line %lu (from %lu): %s", __LINE__, from, #e); \ + } \ } while (0) -#define do_test1(e, msg) \ - do { \ - if (e) { ; } else { err(L"Test failed on line %lu: %ls", __LINE__, (msg)); } \ +#define do_test1(e, msg) \ + do { \ + if (e) { \ + ; \ + } else { \ + err(L"Test failed on line %lu: %ls", __LINE__, (msg)); \ + } \ } while (0) +/// Test that the fish functions for converting strings to numbers work. +static void test_str_to_num() { + const wchar_t *end; + int i; + long l; + + i = fish_wcstoi(L""); + do_test1(errno == EINVAL && i == 0, L"converting empty string to int did not fail"); + i = fish_wcstoi(L" \n "); + do_test1(errno == EINVAL && i == 0, L"converting whitespace string to int did not fail"); + i = fish_wcstoi(L"123"); + do_test1(errno == 0 && i == 123, L"converting valid num to int did not succeed"); + i = fish_wcstoi(L"-123"); + do_test1(errno == 0 && i == -123, L"converting valid num to int did not succeed"); + i = fish_wcstoi(L" 345 "); + do_test1(errno == 0 && i == 345, L"converting valid num to int did not succeed"); + i = fish_wcstoi(L" -345 "); + do_test1(errno == 0 && i == -345, L"converting valid num to int did not succeed"); + i = fish_wcstoi(L"x345"); + do_test1(errno == EINVAL && i == 0, L"converting invalid num to int did not fail"); + i = fish_wcstoi(L" x345"); + do_test1(errno == EINVAL && i == 0, L"converting invalid num to int did not fail"); + i = fish_wcstoi(L"456 x"); + do_test1(errno == -1 && i == 456, L"converting invalid num to int did not fail"); + i = fish_wcstoi(L"99999999999999999999999"); + do_test1(errno == ERANGE && i == INT_MAX, L"converting invalid num to int did not fail"); + i = fish_wcstoi(L"-99999999999999999999999"); + do_test1(errno == ERANGE && i == INT_MIN, L"converting invalid num to int did not fail"); + i = fish_wcstoi(L"567]", &end); + do_test1(errno == -1 && i == 567 && *end == L']', + L"converting valid num to int did not succeed"); + // This is subtle. "567" in base 8 is "375" in base 10. The final "8" is not converted. + i = fish_wcstoi(L"5678", &end, 8); + do_test1(errno == -1 && i == 375 && *end == L'8', + L"converting invalid num to int did not fail"); + + l = fish_wcstol(L""); + do_test1(errno == EINVAL && l == 0, L"converting empty string to long did not fail"); + l = fish_wcstol(L" \t "); + do_test1(errno == EINVAL && l == 0, L"converting whitespace string to long did not fail"); + l = fish_wcstol(L"123"); + do_test1(errno == 0 && l == 123, L"converting valid num to long did not succeed"); + l = fish_wcstol(L"-123"); + do_test1(errno == 0 && l == -123, L"converting valid num to long did not succeed"); + l = fish_wcstol(L" 345 "); + do_test1(errno == 0 && l == 345, L"converting valid num to long did not succeed"); + l = fish_wcstol(L" -345 "); + do_test1(errno == 0 && l == -345, L"converting valid num to long did not succeed"); + l = fish_wcstol(L"x345"); + do_test1(errno == EINVAL && l == 0, L"converting invalid num to long did not fail"); + l = fish_wcstol(L" x345"); + do_test1(errno == EINVAL && l == 0, L"converting invalid num to long did not fail"); + l = fish_wcstol(L"456 x"); + do_test1(errno == -1 && l == 456, L"converting invalid num to long did not fail"); + l = fish_wcstol(L"99999999999999999999999"); + do_test1(errno == ERANGE && l == LONG_MAX, L"converting invalid num to long did not fail"); + l = fish_wcstol(L"-99999999999999999999999"); + do_test1(errno == ERANGE && l == LONG_MIN, L"converting invalid num to long did not fail"); + l = fish_wcstol(L"567]", &end); + do_test1(errno == -1 && l == 567 && *end == L']', + L"converting valid num to long did not succeed"); + // This is subtle. "567" in base 8 is "375" in base 10. The final "8" is not converted. + l = fish_wcstol(L"5678", &end, 8); + do_test1(errno == -1 && l == 375 && *end == L'8', + L"converting invalid num to long did not fail"); +} + /// Test sane escapes. static void test_unescape_sane() { const struct test_t { @@ -1008,10 +1088,10 @@ static void test_utf8() { "um/wm boundaries +1"); test_utf82wchar(um, sizeof(um), NULL, 0, 0, sizeof(wm) / sizeof(*wm), "um/wm calculate length"); - // The following tests won't pass on systems (e.g., Cygwin) where sizeof wchar_t is 2. That's - // due to several reasons but the primary one is that narrowing conversions of literals assigned - // to the wchar_t arrays above don't result in values that will be treated as errors by the - // conversion functions. +// The following tests won't pass on systems (e.g., Cygwin) where sizeof wchar_t is 2. That's +// due to several reasons but the primary one is that narrowing conversions of literals assigned +// to the wchar_t arrays above don't result in values that will be treated as errors by the +// conversion functions. #if WCHAR_MAX != 0xffff test_utf82wchar(u4, sizeof(u4), w4, sizeof(w4) / sizeof(*w4), 0, sizeof(w4) / sizeof(*w4), "u4/w4 4 octets chars"); @@ -2827,19 +2907,17 @@ void history_tests_t::test_history_merge(void) { } // Make sure incorporate_external_changes doesn't drop items! (#3496) - history_t * const writer = hists[0]; - history_t * const reader = hists[1]; - const wcstring more_texts[] = { - L"Item_#3496_1", L"Item_#3496_2", L"Item_#3496_3", L"Item_#3496_4", - L"Item_#3496_5", L"Item_#3496_6" - }; - for (size_t i=0; i < sizeof more_texts / sizeof *more_texts; i++) { + history_t *const writer = hists[0]; + history_t *const reader = hists[1]; + const wcstring more_texts[] = {L"Item_#3496_1", L"Item_#3496_2", L"Item_#3496_3", + L"Item_#3496_4", L"Item_#3496_5", L"Item_#3496_6"}; + for (size_t i = 0; i < sizeof more_texts / sizeof *more_texts; i++) { // time_barrier because merging will ignore items that may be newer if (i > 0) time_barrier(); writer->add(more_texts[i]); writer->incorporate_external_changes(); reader->incorporate_external_changes(); - for (size_t j=0; j < i; j++) { + for (size_t j = 0; j < i; j++) { do_test(history_contains(reader, more_texts[j])); } } @@ -3114,8 +3192,7 @@ static bool test_1_parse_ll2(const wcstring &src, wcstring *out_cmd, wcstring *o tree.command_for_plain_statement(stmt, src, out_cmd); // Return arguments separated by spaces. - const parse_node_tree_t::parse_node_list_t arg_nodes = - tree.find_nodes(stmt, symbol_argument); + const parse_node_tree_t::parse_node_list_t arg_nodes = tree.find_nodes(stmt, symbol_argument); for (size_t i = 0; i < arg_nodes.size(); i++) { if (i > 0) out_joined_args->push_back(L' '); out_joined_args->append(arg_nodes.at(i)->get_source(src)); @@ -3959,6 +4036,7 @@ int main(int argc, char **argv) { // Set default signal handlers, so we can ctrl-C out of this. signal_reset_handlers(); + if (should_test_function("str_to_num")) test_str_to_num(); if (should_test_function("highlighting")) test_highlighting(); if (should_test_function("new_parser_ll2")) test_new_parser_ll2(); if (should_test_function("new_parser_fuzzing")) diff --git a/src/highlight.cpp b/src/highlight.cpp index 85844dc1c..97962434c 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -4,11 +4,9 @@ // IWYU pragma: no_include #include #include -#include #include #include #include -#include #include #include #include @@ -874,19 +872,8 @@ void highlighter_t::color_redirection(const parse_node_t &redirection_node) { path_apply_working_directory(target, this->working_directory); switch (redirect_type) { case TOK_REDIRECT_FD: { - // Target should be an fd. It must be all digits, and must not overflow. - // fish_wcstoi returns INT_MAX on overflow; we could instead check errno to - // disambiguiate this from a real INT_MAX fd, but instead we just disallow - // that. - const wchar_t *target_cstr = target.c_str(); - wchar_t *end = NULL; - int fd = fish_wcstoi(target_cstr, &end, 10); - - // The iswdigit check ensures there's no leading whitespace, the *end check - // ensures the entire string was consumed, and the numeric checks ensure the - // fd is at least zero and there was no overflow. - target_is_valid = - (iswdigit(target_cstr[0]) && *end == L'\0' && fd >= 0 && fd < INT_MAX); + int fd = fish_wcstoi(target.c_str()); + target_is_valid = !errno && fd >= 0; break; } case TOK_REDIRECT_IN: { diff --git a/src/history.cpp b/src/history.cpp index c48163a7f..13539fe9e 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -308,16 +308,10 @@ static history_item_t decode_item_fish_1_x(const char *begin, size_t length) { if (timestamp_mode) { const wchar_t *time_string = out.c_str(); while (*time_string && !iswdigit(*time_string)) time_string++; - errno = 0; if (*time_string) { - time_t tm; - wchar_t *end; - - errno = 0; - tm = (time_t)wcstol(time_string, &end, 10); - - if (tm && !errno && !*end) { + time_t tm = (time_t)fish_wcstol(time_string); + if (!errno && tm >= 0) { timestamp = tm; } } @@ -959,8 +953,8 @@ bool history_t::map_file(const wcstring &name, const char **out_map_start, size_ size_t mmap_length = (size_t)len; if (lseek(fd, 0, SEEK_SET) == 0) { char *mmap_start; - if ((mmap_start = (char *)mmap(0, mmap_length, PROT_READ, MAP_PRIVATE, fd, - 0)) != MAP_FAILED) { + if ((mmap_start = (char *)mmap(0, mmap_length, PROT_READ, MAP_PRIVATE, fd, 0)) != + MAP_FAILED) { result = true; *out_map_start = mmap_start; *out_map_len = mmap_length; diff --git a/src/input_common.cpp b/src/input_common.cpp index a82e801ce..7a2ff24c9 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -25,6 +25,7 @@ #include "input_common.h" #include "iothread.h" #include "util.h" +#include "wutil.h" /// Time in milliseconds to wait for another byte to be available for reading /// after \x1b is read before assuming that escape key was pressed, and not an @@ -167,13 +168,11 @@ void update_wait_on_escape_ms() { return; } - wchar_t *endptr; - long tmp = wcstol(escape_time_ms.c_str(), &endptr, 10); - - if (*endptr != '\0' || tmp < 10 || tmp >= 5000) { + long tmp = fish_wcstol(escape_time_ms.c_str()); + if (errno || tmp < 10 || tmp >= 5000) { fwprintf(stderr, L"ignoring fish_escape_delay_ms: value '%ls' " - "is not an integer or is < 10 or >= 5000 ms\n", + L"is not an integer or is < 10 or >= 5000 ms\n", escape_time_ms.c_str()); } else { wait_on_escape_ms = (int)tmp; diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index b18514ef2..1297a2aee 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -399,7 +399,7 @@ parse_execution_result_t parse_execution_context_t::run_function_statement( wcstring error_str; io_streams_t streams; int err = builtin_function(*parser, streams, argument_list, contents_str, - definition_line_offset, &error_str); + definition_line_offset, &error_str); proc_set_last_status(err); if (!error_str.empty()) { @@ -1010,10 +1010,8 @@ bool parse_execution_context_t::determine_io_chain(const parse_node_t &statement if (target == L"-") { new_io.reset(new io_close_t(source_fd)); } else { - wchar_t *end = NULL; - errno = 0; - int old_fd = fish_wcstoi(target.c_str(), &end, 10); - if (old_fd < 0 || errno || *end) { + int old_fd = fish_wcstoi(target.c_str()); + if (errno || old_fd < 0) { errored = report_error(redirect_node, _(L"Requested redirection to '%ls', which " L"is not a valid file descriptor"), diff --git a/src/signal.cpp b/src/signal.cpp index 46e501b81..9d48d1121 100644 --- a/src/signal.cpp +++ b/src/signal.cpp @@ -150,19 +150,15 @@ static int match_signal_name(const wchar_t *canonical, const wchar_t *name) { } int wcs2sig(const wchar_t *str) { - int i; - wchar_t *end = 0; - - for (i = 0; lookup[i].desc; i++) { + for (int i = 0; lookup[i].desc; i++) { if (match_signal_name(lookup[i].name, str)) { return lookup[i].signal; } } - errno = 0; - int res = fish_wcstoi(str, &end, 10); - if (!errno && res >= 0 && !*end) return res; - return -1; + int res = fish_wcstoi(str); + if (errno || res < 0) return -1; + return res; } const wchar_t *sig2wcs(int sig) { diff --git a/src/util.cpp b/src/util.cpp index 7caba32bc..600ef9e36 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -28,16 +28,15 @@ int wcsfilecmp(const wchar_t *a, const wchar_t *b) { long secondary_diff = 0; if (iswdigit(*a) && iswdigit(*b)) { - wchar_t *aend, *bend; + const wchar_t *aend, *bend; long al; long bl; long diff; - errno = 0; - al = wcstol(a, &aend, 10); - bl = wcstol(b, &bend, 10); - - if (errno) { + al = fish_wcstol(a, &aend); + int a1_errno = errno; + bl = fish_wcstol(b, &bend); + if (a1_errno || errno) { // Huge numbers - fall back to regular string comparison. return wcscmp(a, b); } diff --git a/src/wutil.cpp b/src/wutil.cpp index 2c12068cb..3cd7fbdfe 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -524,16 +524,74 @@ int fish_wcswidth(const wchar_t *str) { return fish_wcswidth(str, wcslen(str)); /// See fallback.h for the normal definitions. int fish_wcswidth(const wcstring &str) { return fish_wcswidth(str.c_str(), str.size()); } -int fish_wcstoi(const wchar_t *str, wchar_t **endptr, int base) { - long ret = wcstol(str, endptr, base); - if (ret > INT_MAX) { - ret = INT_MAX; +/// Like fish_wcstol(), but fails on a value outside the range of an int. +/// +/// This is needed because BSD and GNU implementations differ in several ways that make it really +/// annoying to use them in a portable fashion. +/// +/// The caller doesn't have to zero errno. Sets errno to -1 if the int ends with something other +/// than a digit. Leading whitespace is ignored (per the base wcstol implementation). Trailing +/// whitespace is also ignored. We also treat empty strings and strings containing only whitespace +/// as invalid. +int fish_wcstoi(const wchar_t *str, const wchar_t **endptr, int base) { + while (iswspace(*str)) ++str; // skip leading whitespace + if (!*str) { // this is because some implementations don't handle this sensibly + errno = EINVAL; + if (endptr) *endptr = str; + return 0; + } + + errno = 0; + wchar_t *_endptr; + long result = wcstol(str, &_endptr, base); + if (result > INT_MAX) { + result = INT_MAX; errno = ERANGE; - } else if (ret < INT_MIN) { - ret = INT_MIN; + } else if (result < INT_MIN) { + result = INT_MIN; errno = ERANGE; } - return (int)ret; + while (iswspace(*_endptr)) ++_endptr; // skip trailing whitespace + if (!errno && *_endptr) { + if (_endptr == str) { + errno = EINVAL; + } else { + errno = -1; + } + } + if (endptr) *endptr = _endptr; + return (int)result; +} + +/// An enhanced version of wcstol(). +/// +/// This is needed because BSD and GNU implementations differ in several ways that make it really +/// annoying to use them in a portable fashion. +/// +/// The caller doesn't have to zero errno. Sets errno to -1 if the int ends with something other +/// than a digit. Leading whitespace is ignored (per the base wcstol implementation). Trailing +/// whitespace is also ignored. +long fish_wcstol(const wchar_t *str, const wchar_t **endptr, int base) { + while (iswspace(*str)) ++str; // skip leading whitespace + if (!*str) { // this is because some implementations don't handle this sensibly + errno = EINVAL; + if (endptr) *endptr = str; + return 0; + } + + errno = 0; + wchar_t *_endptr; + long result = wcstol(str, &_endptr, base); + while (iswspace(*_endptr)) ++_endptr; // skip trailing whitespace + if (!errno && *_endptr) { + if (_endptr == str) { + errno = EINVAL; + } else { + errno = -1; + } + } + if (endptr) *endptr = _endptr; + return result; } file_id_t file_id_t::file_id_from_stat(const struct stat *buf) { diff --git a/src/wutil.h b/src/wutil.h index 1bd17f6f3..88ec1ed08 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -117,8 +117,8 @@ bool wcsvarchr(wchar_t chr); int fish_wcswidth(const wchar_t *str); int fish_wcswidth(const wcstring &str); -/// Like wcstol(), but fails on a value outside the range of an int. -int fish_wcstoi(const wchar_t *str, wchar_t **endptr, int base); +int fish_wcstoi(const wchar_t *str, const wchar_t **endptr = NULL, int base = 10); +long fish_wcstol(const wchar_t *str, const wchar_t **endptr = NULL, int base = 10); /// Class for representing a file's inode. We use this to detect and avoid symlink loops, among /// other things. While an inode / dev pair is sufficient to distinguish co-existing files, Linux