New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-42008: Fix _random.Random() seeding #22668
bpo-42008: Fix _random.Random() seeding #22668
Conversation
Modules/_randommodule.c
Outdated
| Py_DECREF(self); | ||
| return NULL; | ||
|
|
||
| if (PyTuple_GET_SIZE(args) == 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong, but afaik, random_new is called with args of _random.Random's init, which can contain more than 1 argument. So it might be more robust to check for len(args) > 0 instead.
>>> import random
>>> random.Random(1,2,3)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: Random.__init__() takes from 1 to 2 positional arguments but 3 were given
>>>
>>> import _random
>>> _random.Random(1,2,3)
>>> # no error| if (PyTuple_GET_SIZE(args) == 1) { | |
| if (PyTuple_GET_SIZE(args) > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fidget-Spinner thanks for the review!
I think, in previous versions (e.g. 3.6) it just accepts one argument. So I'm not sure if it is a good behavior for random_new to accept more arguments or not! (I think, it should raise an exception in case of being called with 2 or more args)
After this commit it is allowed to pass more arguments (don't know it's a bug or feature).
Dear @rhettinger, do you have any idea about it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only accept one argument. If there are more, raise a TypeError.
|
Changed the code to only accept 0 or 1 argument. Also if it uses an empty tuple as seed (not current time). This problem also solved by this PR. |
|
This PR is stale because it has been open for 30 days with no activity. |
|
Can you please resolve the branch conflict. |
|
Sure |
|
Actually I think Azure Pipeline failure is not related to my changes |
calling random_seed() with first element of the args tuple.
https://bugs.python.org/issue42008