Re: [PATCH] zsh/random module [UPDATED]

[Clinton Bunch <cdb_zsh@xxxxxxxxxxx>]

On 11/23/2022 1:46 PM, Daniel Shahaf wrote:
> Clinton Bunch wrote on Mon, Nov 07, 2022 at 18:18:15 -0600:

[snip]
> > +++ b/Src/Modules/random.c
> > @@ -0,0 +1,675 @@
> There's a whole bunch of orthographic issues in this file (whitespace,
> punctuation, spelling, etc.).  I won't point them out now since there
> are bigger issues.

The default color scheme in vim syntax highlighting makes comments very 
hard to read, so I ended up not proofreading them as much as I should have.

The whitespace issues are due to trying to write in, for me, an 
unnatural coding style.  I'm working on it.  I've seen inconsistent 
coding styles elsewhere in the code, perhaps a code formatter should be 
recommended with appropriate options.  The editconfig file is helpful, 
but doesn't work well when lines are added or  a new indentation level 
is required for existing code. (When writing Perl code, I'm a big fan of 
perltidy)

[snip]

> > +    bool integer_out = false;	 /*  boolean */
> The target audience for comments knows C.  So, nuke this comment.
The comment is left over from when integer_out was an int, before I was 
told C99 was acceptable and changed it to bool.[snip]
> > +    /* Vailues for -U *nd -L */
> > +    zulong   upper = UINT32_MAX, lower = 0;
> > +    uint32_t diff  = UINT32_MAX;
> > +    uint32_t min   = 0, rej = 0;  /* Reject below min and count */
> «rej» is never read.
It should probably be wrapped in an #ifdef DEBUG as should its intended 
use. (I used it track how many numbers were being discarded to achieve 
uniform distribution during debugging. I removed those statements and 
forgot to remove the declaration.  I'm surprised I didn't get a warning)
>


[snip]

> > +/**/
> > +uint64_t
> > +random64(void) {
> > +    uint64_t r;
> > +
> > +    if(getrandom_buffer(&r,sizeof(r)) < 0) {
> > +	zwarn("zsh/random: Can't get sufficient random data.");
> > +	return 1; /* 0 will cause loop */
> Where is the docstring of random64() itself?  /That/ is what determines
> whether or not returning 0 would be correct.
>
> What loops and why?

The loop in the random_real that checks for repeated zeros and 
terminates when the exponent reaches the minimum value.

[snip]


> Thanks,
>
> Daniel