diff --git a/doc_src/random.txt b/doc_src/random.txt index 5d8a8bab3..84637df10 100644 --- a/doc_src/random.txt +++ b/doc_src/random.txt @@ -2,31 +2,40 @@ \subsection random-synopsis Synopsis \fish{synopsis} -random [SEED] +random +random SEED +random START END +random START STEP END +random choice [ITEMS...] \endfish \subsection random-description Description -`random` outputs a psuedo-random number from 0 to 32767, inclusive. -Even ignoring the very narrow range of values you should not assume -this produces truly random values within that range. Do not use the -value for any cryptographic purposes, and take care to handle collisions: -the same random number appearing more than once in a given fish instance. +`RANDOM` generates a pseudo-random integer from a uniform distribution. The +range (inclusive) is dependent on the arguments passed. +No arguments indicate a range of [0; 32767]. +If one argument is specified, the internal engine will be seeded with the +argument for future invocations of `RANDOM` and no output will be produced. +Two arguments indicate a range of [START; END]. +Three arguments indicate a range of [START; END] with a spacing of STEP +between possible outputs. +`RANDOM choice` will select one random item from the succeeding arguments. -If a `SEED` value is provided, it is used to seed the random number -generator, and no output will be produced. This can be useful for debugging -purposes, where it can be desirable to get the same random number sequence -multiple times. If the random number generator is called without first -seeding it, the current time will be used as the seed. +Note that seeding the engine will NOT give the same result across different +systems. +You should not consider `RANDOM` cryptographically secure, or even +statistically accurate. \subsection random-example Example -The following code will count down from a random number to 1: - +The following code will count down from a random even number between 10 and 20 to 1: \fish -for i in (seq (random) -1 1) +for i in (seq (random 10 2 20) -1 1) echo $i - sleep end \endfish +And this will open a random picture from any of the subdirectories: +\fish +open (random choice **jpg) +\endfish diff --git a/src/builtin.cpp b/src/builtin.cpp index d8d10c5e2..b3d6ee2d4 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -26,12 +26,13 @@ #include #include #include -#include +#include #include #include #include #include #include +#include #include #include @@ -1742,15 +1743,21 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis /// The random builtin generates random numbers. static int builtin_random(parser_t &parser, io_streams_t &streams, wchar_t **argv) { - static int seeded = 0; - static struct drand48_data seed_buffer; - - int argc = builtin_count_args(argv); + static bool seeded = false; + static std::minstd_rand engine; + if (!seeded) { + // seed engine with 2*32 bits of random data + // for the 64 bits of internal state of minstd_rand + std::random_device rd; + std::seed_seq seed{rd(), rd()}; + engine.seed(seed); + seeded = true; + } wgetopter_t w; - - static const struct woption long_options[] = {{L"help", no_argument, 0, 'h'}, {0, 0, 0, 0}}; - + int argc = builtin_count_args(argv); + static const struct woption long_options[] = {{L"help", no_argument, NULL, 'h'}, + {NULL, 0, NULL, 0}}; while (1) { int opt_index = 0; @@ -1767,7 +1774,7 @@ static int builtin_random(parser_t &parser, io_streams_t &streams, wchar_t **arg } case 'h': { builtin_print_help(parser, streams, argv[0], streams.out); - break; + return STATUS_BUILTIN_OK; } case '?': { builtin_unknown_option(parser, streams, argv[0], argv[w.woptind - 1]); @@ -1781,29 +1788,115 @@ static int builtin_random(parser_t &parser, io_streams_t &streams, wchar_t **arg } int arg_count = argc - w.woptind; - if (arg_count == 0) { - long res; - if (!seeded) { - seeded = 1; - srand48_r(time(0), &seed_buffer); - } - lrand48_r(&seed_buffer, &res); - streams.out.append_format(L"%ld\n", res % 32768); - } else if (arg_count == 1) { - 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]); + long long start, end; + unsigned long long step; + bool choice = false; + if (arg_count >= 1 && !wcscmp(argv[w.woptind], L"choice")) { + if (arg_count == 1) { + streams.err.append_format(L"%ls: nothing to choose from\n", argv[0]); return STATUS_BUILTIN_ERROR; } - seeded = 1; - srand48_r(foo, &seed_buffer); + choice = true; + start = 1; + step = 1; + end = arg_count - 1; } else { - streams.err.append_format(_(L"%ls: Expected zero or one argument, got %d\n"), argv[0], - argc - w.woptind); - builtin_print_help(parser, streams, argv[0], streams.err); + bool parse_error = false; + auto parse_ll = [&](const wchar_t *str) { + long long ll = fish_wcstoll(str); + if (errno) { + streams.err.append_format(L"%ls: %ls is not a valid integer\n", argv[0], str); + parse_error = true; + } + return ll; + }; + auto parse_ull = [&](const wchar_t *str) { + unsigned long long ull = fish_wcstoull(str); + if (errno) { + streams.err.append_format(L"%ls: %ls is not a valid integer\n", argv[0], str); + parse_error = true; + } + return ull; + }; + if (arg_count == 0) { + start = 0; + end = 32767; + step = 1; + } else if (arg_count == 1) { + long long seed = parse_ll(argv[w.woptind]); + if (parse_error) return STATUS_BUILTIN_ERROR; + engine.seed(static_cast(seed)); + return STATUS_BUILTIN_OK; + } else if (arg_count == 2) { + start = parse_ll(argv[w.woptind]); + step = 1; + end = parse_ll(argv[w.woptind + 1]); + } else if (arg_count == 3) { + start = parse_ll(argv[w.woptind]); + step = parse_ull(argv[w.woptind + 1]); + end = parse_ll(argv[w.woptind + 2]); + } else { + streams.err.append_format(BUILTIN_ERR_TOO_MANY_ARGUMENTS, argv[0]); + return STATUS_BUILTIN_ERROR; + } + + if (parse_error) { + return STATUS_BUILTIN_ERROR; + } else if (start >= end) { + streams.err.append_format(L"%ls: END must be greater than START\n", argv[0]); + return STATUS_BUILTIN_ERROR; + } else if (step == 0) { + streams.err.append_format(L"%ls: STEP must be a positive integer\n", argv[0]); + return STATUS_BUILTIN_ERROR; + } + } + + // only for negative argument + auto safe_abs = [](long long ll) -> unsigned long long { + return -static_cast(ll); + }; + long long real_end; + if (start >= 0 || end < 0) { + // 0 <= start <= end + long long diff = end - start; + // 0 <= diff <= LL_MAX + real_end = start + static_cast(diff / step); + } else { + // start < 0 <= end + unsigned long long abs_start = safe_abs(start); + unsigned long long diff = (end + abs_start); + real_end = diff / step - abs_start; + } + + if (!choice && start == real_end) { + streams.err.append_format(L"%ls: range contains only one possible value\n", argv[0]); return STATUS_BUILTIN_ERROR; } + + std::uniform_int_distribution dist(start, real_end); + long long random = dist(engine); + long long result; + if (start >= 0) { + // 0 <= start <= random <= end + long long diff = random - start; + // 0 < step * diff <= end - start <= LL_MAX + result = start + static_cast(diff * step); + } else if (random < 0) { + // start <= random < 0 + long long diff = random - start; + result = diff * step - safe_abs(start); + } else { + // start < 0 <= random + unsigned long long abs_start = safe_abs(start); + unsigned long long diff = (random + abs_start); + result = diff * step - abs_start; + } + + if (choice) { + streams.out.append_format(L"%ls\n", argv[w.woptind + result]); + } else { + streams.out.append_format(L"%lld\n", result); + } return STATUS_BUILTIN_OK; } diff --git a/src/wutil.cpp b/src/wutil.cpp index 9aa76102b..3d8c9145f 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -631,11 +631,13 @@ long long fish_wcstoll(const wchar_t *str, const wchar_t **endptr, int base) { /// 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 wcstoll implementation). Trailing -/// whitespace is also ignored. +/// than a digit. Leading minus is considered invalid. Leading whitespace is ignored (per the base +/// wcstoull implementation). Trailing whitespace is also ignored. unsigned long long fish_wcstoull(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 + if (!*str || // this is because some implementations don't handle this sensibly + *str == '-') // disallow minus as the first character to avoid questionable wrap-around + { errno = EINVAL; if (endptr) *endptr = str; return 0; diff --git a/tests/random.err b/tests/random.err new file mode 100644 index 000000000..d6f1ea51d --- /dev/null +++ b/tests/random.err @@ -0,0 +1,16 @@ +random: a is not a valid integer +random: 18446744073709551614 is not a valid integer +random: Too many arguments +random: END must be greater than START +random: 18446744073709551614 is not a valid integer +random: 1d is not a valid integer +random: 1c is not a valid integer +random: END must be greater than START +random: - is not a valid integer +random: -1 is not a valid integer +random: -9223372036854775807 is not a valid integer +random: STEP must be a positive integer +random: range contains only one possible value +random: range contains only one possible value +random: nothing to choose from +random: Too many arguments diff --git a/tests/random.in b/tests/random.in new file mode 100644 index 000000000..d8e31df19 --- /dev/null +++ b/tests/random.in @@ -0,0 +1,98 @@ +set -l max 9223372036854775807 +set -l close_max 9223372036854775806 +set -l min -9223372036854775807 +set -l close_min -9223372036854775806 +set -l diff_max 18446744073709551614 + +# check failure cases +random a +random $diff_max +random -- 1 2 3 4 +random -- 10 -10 +random -- 10 $diff_max +random -- 1 1d +random -- 1 1c 10 +random -- 10 10 +random -- 1 - 10 +random -- 1 -1 10 +random -- 1 $min 10 +random -- 1 0 10 +random -- 1 11 10 +random -- 0 $diff_max $max +random choice +random choic a b c + +function check_boundaries + if not test $argv[1] -ge $argv[2] -a $argv[1] -le $argv[3] + printf "Unexpected: %s <= %s <= %s not verified\n" $argv[2] $argv[1] $argv[3] >&2 + return 1 + end +end + +function test_range + return (check_boundaries (random -- $argv) $argv) +end + +function check_contains + if not contains -- $argv[1] $argv[2..-1] + printf "Unexpected: %s not among possibilities" $argv[1] >&2 + printf " %s" $argv[2..-1] >&2 + printf "\n" >&2 + return 1 + end +end + +function test_step + return (check_contains (random -- $argv) (seq -- $argv)) +end + +function test_choice + return (check_contains (random choice $argv) $argv) +end + +for i in (seq 10) + check_boundaries (random) 0 32767 + + test_range 0 10 + test_range -10 -1 + test_range -10 10 + + test_range 0 $max + test_range $min -1 + test_range $min $max + + test_range $close_max $max + test_range $min $close_min + test_range $close_min $close_max + + #OSX's `seq` uses scientific notation for large numbers, hence not usable here + check_contains (random -- 0 $max $max) 0 $max + check_contains (random -- 0 $close_max $max) 0 $close_max + check_contains (random -- $min $max 0) $min 0 + check_contains (random -- $min $close_max 0) $min -1 + check_contains (random -- $min $max $max) $min 0 $max + check_contains (random -- $min $diff_max $max) $min $max + + test_step 0 $i 10 + test_step -5 $i 5 + test_step -10 $i 0 + + test_choice a + test_choice foo bar + test_choice bass trout salmon zander perch carp +end + + +#check seeding +set -l seed (random) +random $seed +set -l run1 (random) (random) (random) (random) (random) +random $seed +set -l run2 (random) (random) (random) (random) (random) +if not test "$run1" = "$run2" + printf "Unexpected different sequences after seeding with %s\n" $seed + printf "%s " $run1 + printf "\n" + printf "%s " $run2 + printf "\n" +end diff --git a/tests/random.out b/tests/random.out new file mode 100644 index 000000000..e69de29bb