Issue3708
Created on 2008-08-27 23:30 by ajaksu2, last changed 2008-09-18 22:53 by rpetrov.
| File name |
Uploaded |
Description |
Edit |
Remove |
|
urandom.diff
|
ajaksu2,
2008-08-27 23:30
|
Check len(bytes) against int(n) |
|
|
|
urandom-float-and-close.diff
|
gregory.p.smith,
2008-08-28 06:00
|
fix os.urandom infinite loop when passed a non integral float |
|
|
| msg72050 (view) |
Author: Daniel Diniz (ajaksu2) |
Date: 2008-08-27 23:30 |
|
Calling os.urandom(1 + float(x)) ends in a infinite loop due to a naive
condition check:
while len(bytes) < n:
bytes += read(_urandomfd, n - len(bytes))
Trivial patch attached.
|
| msg72064 (view) |
Author: Gregory P. Smith (gregory.p.smith) |
Date: 2008-08-28 05:42 |
|
i'll fix this and add a unit test.
|
| msg72065 (view) |
Author: Gregory P. Smith (gregory.p.smith) |
Date: 2008-08-28 06:00 |
|
better patch with tests attached, no explicit int conversion is done.
i also wrapped the use of the fd returned by open with a try: finally:
to avoid any chance of a leak and renamed the bytes variable to bs to
match whats in py3k and avoid overriding the builtin type.
|
| msg72074 (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-28 09:40 |
|
The explicit int() conversion looked saner to me, rather than passing a
float argument to read().
By the way, is there a reason this code still uses os.open rather than
the builtin io.open?
|
| msg72107 (view) |
Author: Gregory P. Smith (gregory.p.smith) |
Date: 2008-08-28 19:30 |
|
if i did
n = int(n)
that would change the API to allow bytes/unicode to be passed in which
is not something i want. i don't even like that it allows floats.
by not doing the int conversion at all, a DeprecationWarning is raised
by the read() about the argument being a float which I figure it not a
bad thing given that the API really should only accept ints...
fwiw, daniel's patch would still cause this deprecation warning from
read so I guess using while len(bs) < int(n): isn't that bad either.
but it would allow a string such as '0' to be passed in as an argument
without an exception...
|
| msg72109 (view) |
Author: Daniel Diniz (ajaksu2) |
Date: 2008-08-28 19:50 |
|
Gregory,
IMHO your patch is better in all aspects.
Regarding my patch, the API wouldn't change at all, as the source reads:
while len(bytes) < int(n):
bytes += read(_urandomfd, n - len(bytes))
So "n - len(bytes)" restricts the API to what it was before. But it
would call int() for each loop iteration :/
@Pitrou: My patch still passed the float to read (to keep the current
behavior, warning included), but if doing that can be considered a bug,
let's get rid of the DeprecationWarning by passing an int.
|
| msg72117 (view) |
Author: Antoine Pitrou (pitrou) |
Date: 2008-08-29 00:25 |
|
Le jeudi 28 août 2008 à 19:30 +0000, Gregory P. Smith a écrit :
> if i did
>
> n = int(n)
>
> that would change the API to allow bytes/unicode to be passed in which
> is not something i want.
Ok, I hadn't thought about that. You convinced me.
|
| msg72291 (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) |
Date: 2008-09-01 20:08 |
|
The patch looks fine to me as well.
|
| msg72314 (view) |
Author: Gregory P. Smith (gregory.p.smith) |
Date: 2008-09-02 05:36 |
|
committed to trunk r66142
|
| msg72791 (view) |
Author: Benjamin Peterson (benjamin.peterson) |
Date: 2008-09-08 21:46 |
|
Gregory, could you merge this into py3k, please?
|
| msg72794 (view) |
Author: Benjamin Peterson (benjamin.peterson) |
Date: 2008-09-08 22:06 |
|
Apparently this isn't an issue in py3k, so no worries! :)
|
| msg73407 (view) |
Author: Roumen Petrov (rpetrov) |
Date: 2008-09-18 22:53 |
|
It seems to me that test case will fail on windows and vms platforms.
The case contain os.urandom(1.1) but in posixmodule.c for urandon
functions (windows and vms) exits:
PyArg_ParseTuple(args, "i:urandom", &howMany))
./python.exe -c 'import os; print "%s" %(os.urandom(1.9))' =>
-c:1: DeprecationWarning: integer argument expected, got float
|
|
| Date |
User |
Action |
Args |
| 2008-09-18 22:53:36 | rpetrov | set | nosy:
+ rpetrov messages:
+ msg73407 |
| 2008-09-08 22:06:28 | benjamin.peterson | set | messages:
+ msg72794 |
| 2008-09-08 21:46:38 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages:
+ msg72791 |
| 2008-09-02 05:36:35 | gregory.p.smith | set | status: open -> closed messages:
+ msg72314 |
| 2008-09-01 20:08:59 | amaury.forgeotdarc | set | keywords:
- needs review nosy:
+ amaury.forgeotdarc resolution: accepted messages:
+ msg72291 |
| 2008-08-29 00:25:53 | pitrou | set | messages:
+ msg72117 |
| 2008-08-28 19:50:55 | ajaksu2 | set | messages:
+ msg72109 |
| 2008-08-28 19:30:30 | gregory.p.smith | set | messages:
+ msg72107 |
| 2008-08-28 09:40:31 | pitrou | set | nosy:
+ pitrou messages:
+ msg72074 |
| 2008-08-28 06:00:35 | gregory.p.smith | set | priority: low -> normal keywords:
+ needs review messages:
+ msg72065 files:
+ urandom-float-and-close.diff |
| 2008-08-28 05:42:07 | gregory.p.smith | set | priority: low assignee: gregory.p.smith messages:
+ msg72064 keywords:
+ easy nosy:
+ gregory.p.smith |
| 2008-08-27 23:30:11 | ajaksu2 | create | |
|