Skip to content

Add php_random_int internal API#2170

Closed
lt wants to merge 1 commit intophp:masterfrom
lt:random_int_api
Closed

Add php_random_int internal API#2170
lt wants to merge 1 commit intophp:masterfrom
lt:random_int_api

Conversation

@lt
Copy link
Copy Markdown
Contributor

@lt lt commented Oct 19, 2016

This is the internal API compliment to php_random_bytes

The purpose is to allow extensions and future enhancements that require high quality unbiased random numbers to fetch them without having to implement their own unbiasing algorithm.

This is the internal API compliment to `php_random_bytes`
@lt lt changed the title Add php_random_int internal API Add php_random_int internal API Oct 19, 2016
@lt lt changed the title Add php_random_int internal API Add php_random_int internal API Oct 19, 2016
@nikic
Copy link
Copy Markdown
Member

nikic commented Oct 19, 2016

LGTM.

Ping @krakjoe @dshafik for target version. This only adds additional APIs, is this fine for 7.1?

@krakjoe
Copy link
Copy Markdown
Member

krakjoe commented Oct 19, 2016

It's okay with me, for reasons ... we can talk about them if you like ... or we can leave it at ok ...

@lt
Copy link
Copy Markdown
Contributor Author

lt commented Oct 19, 2016

I'm happy with "ok" ;)

@krakjoe
Copy link
Copy Markdown
Member

krakjoe commented Oct 20, 2016

I think it's too late for 7.0 (@weltling please confirm).

Once confirm is recv'd, please rebase on appropriate branch.

@KalleZ
Copy link
Copy Markdown
Member

KalleZ commented Oct 20, 2016

@krakjoe its a small self contained function, I do not see a reason why this cannot go into 7.0 if it also goes into 7.1, as it may also help transition of 7.0 -> 7.1 if the function is available earlier

@krakjoe
Copy link
Copy Markdown
Member

krakjoe commented Oct 20, 2016

That's a good point.

My early morning brain said that some of the justification for inclusion in 7.1 might not apply to 7.0, but I guess some of it does.

I'd like it in 7.0 whatever.

@lt
Copy link
Copy Markdown
Contributor Author

lt commented Oct 20, 2016

Ok I'll take this positive sentiment, and go ahead and merge.

PHP-7.0: c3361f1
PHP-7.1: f5244f3
master: f23e99d

@lt lt closed this Oct 20, 2016
@weltling
Copy link
Copy Markdown
Contributor

Yeah, that's fine for 7.0. Please merge.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants