zsh-workers
 help / color / mirror / code / Atom feed
From: Matthew Martin <phy1729@gmail.com>
To: Clinton Bunch <cdb_zsh@zentaur.org>
Cc: zsh-workers@zsh.org
Subject: Re: [PATCH] zsh/random module [UPDATED]
Date: Tue, 22 Nov 2022 19:23:05 -0600	[thread overview]
Message-ID: <Y311+V0r6acebcMp@CptOrmolo.darkstar> (raw)
In-Reply-To: <30a7e749-7f30-ecae-6479-a345b1682e7f@zentaur.org>

On Sun, Nov 20, 2022 at 08:57:25PM -0600, Clinton Bunch wrote:
> On 11/20/2022 8:21 PM, Matthew Martin wrote:
> > If a uniform random function is desired in zsh, I think it should mirror
> > the interface of arc4random_uniform: just take an upper_bound and return
> > a value in the range [0, upper_bound).
> 
> The original implementation did exclude the upper bound, inadvertently, but
> after discussion it seemed that was counter-intuitive and something easily
> missed in a cursory glance at the documentation.  Adding a lower bound was
> easy enough and saves extra shell code even if it's not likely to be used as
> much. 

arc4random_uniform excludes the upper bound so it is a direct
replacement for <random source> % <upper bound>. I think it would be
wise to follow an established API and avoid off by one errors when
refactoring existing code. Similarly it doesn't offer a lower bound
option since that's trivial to implement and difficult to get wrong.

> yes, much of getrandom *could* be implemented in shell code, much less
> efficiently.  For example my precmd could save the math eval by specifying
> getrandom -L 1 -U 7.
> 
> Once you've written the code to access the kernel random source, it seems to
> make sense to me to make its output as flexible as possible.  I don't get
> the concern about backwards compatibility in implementing a new builtin
> unless you know of an external command called getrandom that is in common
> use, and that's easily fixed by changing it to zgetrandom.

I disagree on making the output as flexible as possible because there
are already features in zsh to get those formats and the API would need
to be maintained into future releases even if seldom used. I think it
would be wise to start with the minimum interface necessary and then
build off it as uses arise.

Interfacing with libc and the kernel to get random numbers is something
that can only be done in C and SRANDOM builds off an existing API, so
I have no opposition to it. I can accept that uniformly distributed
random integers is a useful and non-trivial task, so probably should be
included, but would prefer an existing and proven API like
arc4random_uniform.

I have a patch based off yours that implements just SRANDOM and
a mathfunc for arc4random_uniform, but haven't yet had time to test it
on multipl platforms.

With just SRANDOM and arc4random_uniform the features of getrandom can
be implemented in a script. I've taken a quick shot at a proof of
concept below.


die() {
	printf >&2 '%s\n' "$1"
	exit 1
}

zmodload zsh/random

local uint32_max=$(( 2**32 - 1 ))
local count=8 lower=0 upper=uint32_max result format=hex output=print output_name
local -a results

while getopts a:c:iL:rs:U: opt; do
	case $opt in
	a) output=array; output_name=$OPTARG;;
	c) count=$OPTARG;;
	L) lower=$OPTARG;;
	i) format=int;;
	r) format=raw; upper=255;;
	s) output=scalar; output_name=$OPTARG; count=1;;
	U) upper=$OPTARG;;
	esac
done

(( count > 0 )) || die 'count must be at least 1'
(( lower >= 0 )) || die 'lower bound must be greater than or equal to 0'
if [[ $format == raw ]]; then
	(( upper <= 255 )) || die 'upper bound must be less than or equal to 255 in raw mode'
else
	(( upper <= uint32_max )) || die "upper bound must be less than or equal to $uint32_max"
fi
(( lower < upper )) || die 'lower bound must be less than upper bound'
if [[ $output == scalar ]] && (( count != 1 )); then die 'count must be 1 for scalar output'; fi

repeat count; do
	if (( lower == 0 && upper == uint32_max )); then
		result=$SRANDOM
	else
		result=$(( arc4random_uniform(upper - lower) + lower ))
	fi

	case $format in
	hex) result=$(( [##16] result ));;
	int) ;;
	raw) printf -v result '%b' "\x$(( [##16] result ))"
	esac

	results+=($result)
done

case $output in
array) : ${(PA)output_name::=$results};;
scalar) : ${(P)output_name::=${results[1]}};;
print) print -rn - $results;;
esac


  parent reply	other threads:[~2022-11-23  1:23 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-02 17:13 [PATCH] zsh/random module Clinton Bunch
2022-11-03 17:50 ` Bart Schaefer
2022-11-04  3:17 ` dana
2022-11-04  6:22   ` Clinton Bunch
2022-11-04  7:27     ` dana
2022-11-04 12:57       ` Clinton Bunch
2022-11-08  0:18         ` [PATCH] zsh/random module [UPDATED] Clinton Bunch
2022-11-18 14:30           ` Clinton Bunch
2022-11-19  6:42             ` Lawrence Velázquez
2022-11-18 16:23           ` Stephane Chazelas
2022-11-18 17:08             ` Clinton Bunch
2022-11-18 18:12               ` Stephane Chazelas
2022-11-18 18:38                 ` Clinton Bunch
2022-11-23 19:52                   ` Daniel Shahaf
2022-11-24 16:19                     ` Stephane Chazelas
2022-11-24 16:30                       ` Roman Perepelitsa
2022-11-24 22:39                         ` Clinton Bunch
2022-11-25  8:53                           ` Stephane Chazelas
2022-11-25  9:40                             ` Stephane Chazelas
2022-11-28 16:37                               ` further discussion of zsh/random (was [PATCH] zsh/random module [UPDATED]) Clinton Bunch
2022-11-21  1:07           ` [PATCH] zsh/random module [UPDATED] Matthew Martin
2022-11-21  1:59             ` Clinton Bunch
2022-11-21  2:21               ` Matthew Martin
2022-11-21  2:57                 ` Clinton Bunch
2022-11-21  3:14                   ` Lawrence Velázquez
2022-11-21  4:17                     ` Bart Schaefer
2022-11-21  5:05                       ` Clinton Bunch
2022-11-22 13:42                         ` dana
2022-11-23 19:49                         ` Daniel Shahaf
2022-11-22 17:44                       ` Oliver Kiddle
2022-11-22 19:48                         ` Clinton Bunch
2022-11-23  1:23                   ` Matthew Martin [this message]
2022-11-23  2:58                     ` Clinton Bunch
2022-11-23  4:14                       ` Matthew Martin
2022-11-23 13:41                         ` Clinton Bunch
2022-11-23 20:33                           ` Daniel Shahaf
2022-11-23 21:42                             ` dana
2022-11-23 23:54                               ` Daniel Shahaf
2022-11-24  0:17                                 ` Daniel Shahaf
2022-11-24  1:05                                 ` dana
2022-11-24 13:52                               ` Clinton Bunch
2022-11-23 19:46           ` Daniel Shahaf
2022-11-24  2:58             ` Clinton Bunch
2022-11-24 10:07               ` nimaje+zml
2022-11-24 13:19                 ` Clinton Bunch
2022-11-24 14:33             ` Clinton Bunch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y311+V0r6acebcMp@CptOrmolo.darkstar \
    --to=phy1729@gmail.com \
    --cc=cdb_zsh@zentaur.org \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).