| 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 |
|
|