konevsa/fish
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

improve converting strings to ints/longs

Browse Source 
The existing code is inconsistent, and in a couple of cases wrong, about
dealing with strings that are not valid ints. For example, there are
locations that call wcstol() and check errno without first setting errno
to zero. Normalize the code to a consistent pattern.  This is mostly to
deal with inconsistencies between BSD, GNU, and other UNIXes.

This does make some syntax more liberal. For example `echo $PATH[1 .. 3]`
is now valid due to uniformly allowing leading and trailing whitespace
around numbers. Whereas prior to this change you would get a "Invalid
index value" error. Contrast this with `echo $PATH[ 1.. 3 ]` which was
valid and still is.
This commit is contained in:
Kurtis Rader 2016-11-22 20:24:03 -08:00
parent 5ec9fcd8d4
commit a928517e95
18 changed files with 266 additions and 215 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
src/builtin.cpp
View File

@ -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;
}

8
src/builtin_commandline.cpp
View File

@ -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);
}

7
src/builtin_jobs.cpp
View File

@ -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;
}

15
src/builtin_set.cpp
View File

@ -192,12 +192,9 @@ static int parse_index(std::vector<long> &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<long> &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) {

28
src/builtin_string.cpp
View File

@ -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;
}

6
src/builtin_ulimit.cpp
View File

@ -4,7 +4,6 @@
#include <errno.h>
#include <stddef.h>
#include <sys/resource.h>
#include <wchar.h>
#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;

17
src/env.cpp
View File

@ -14,7 +14,6 @@
#include <time.h>
#include <unistd.h>
#include <wchar.h>
#include <wctype.h>
#include <algorithm>
#include <map>
#include <set>
@ -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<long>(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.

8
src/exec.cpp
View File

@ -11,13 +11,11 @@
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <wchar.h>
#include <algorithm>
#include <vector>
#ifdef HAVE_SPAWN_H
#include <spawn.h>
#endif
#include <wctype.h>
#include <map>
#include <memory>
#include <string>
@ -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<long>(shlvl_i - 1);
}
}

34
src/expand.cpp
View File

@ -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<long>(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<long> &idx,
std::vector<size_t> &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<long
while (in[pos] == INTERNAL_SEPARATOR) pos++;
const size_t number_start = pos;
long tmp1 = wcstol(&in[pos], &end, 10);
if ((errno) || (end == &in[pos])) {
long tmp1 = fish_wcstol(&in[pos], &end);
// Ignore the case of errno==-1 because it means the end char wasn't the null char.
if (errno > 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<long
}
if (end_ptr) {
// debug( 0, L"Remainder is '%ls', slice def was %d characters long", in+pos, pos );
*end_ptr = (wchar_t *)(in + pos);
}
// debug( 0, L"ok, done" );
return 0;
}

132
src/fish_tests.cpp
View File

@ -4,14 +4,15 @@
// IWYU pragma: no_include <cstring>
// IWYU pragma: no_include <cstddef>
#include <assert.h>
#include <errno.h>
#include <libgen.h>
#include <limits.h>
#include <pthread.h>
#include <signal.h>
#include <stdarg.h>
#include <stddef.h>
#include <stdio.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/select.h>
@ -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"))

17
src/highlight.cpp
View File

@ -4,11 +4,9 @@
// IWYU pragma: no_include <cstddef>
#include <dirent.h>
#include <errno.h>
#include <limits.h>
#include <sys/stat.h>
#include <unistd.h>
#include <wchar.h>
#include <wctype.h>
#include <algorithm>
#include <map>
#include <memory>
@ -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: {

14
src/history.cpp
View File

@ -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;

9
src/input_common.cpp
View File

@ -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;

8
src/parse_execution.cpp
View File

@ -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"),

12
src/signal.cpp
View File

@ -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) {

11
src/util.cpp
View File

@ -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);
}

72
src/wutil.cpp
View File

@ -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) {

4
src/wutil.h
View File

@ -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