classification
Title: py_getrandom() uses an int for syscall() result
Type: Stage:
Components: Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: martin.panter, python-dev, vstinner
Priority: normal Keywords:

Created on 2016-06-09 07:42 by vstinner, last changed 2016-06-16 22:04 by vstinner. This issue is now closed.

Messages (8)
msg267969 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-06-09 07:42
syscall() result type is long.

Moreover, long type might can smaller than the size type, so we may need to add:

n = Py_MIN(size, LONG_MAX);
msg267986 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-06-09 08:16
According to <http://man7.org/linux/man-pages/man2/getrandom.2.html>, getrandom() returns no more than 32 MiB as an int on Linux. Doesn’t that mean you can rely on syscall()’s long return value fitting in an int? Maybe just cast n = (int)sycall(...) to be explicit.

But it does make sense to cap n to LONG_MAX just in case there is some strange platform where it matters :)
msg267989 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-06-09 08:19
Make that INT_MAX. Or change n from an int to a Py_ssize_t. Both Linux and Solaris versions or getrandom() are documented as accepting size_t buflen.
msg268559 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-14 14:35
New changeset e028e86a5b73 by Victor Stinner in branch '3.5':
Fix os.urandom() using getrandom() on Linux
https://hg.python.org/cpython/rev/e028e86a5b73

New changeset 0d39bd9028e8 by Victor Stinner in branch 'default':
Merge 3.5 (os.urandom, issue #27278)
https://hg.python.org/cpython/rev/0d39bd9028e8
msg268560 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-06-14 14:37
Martin: What do you think of my change? Is it enough? Or would you prefer an explicit cast on syscall() result?

I hesitated to use a wider type since the manual page shows an "int" type, not long.
msg268595 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-06-15 00:11
Yeah I think your change is enough.

Adding a cast would solve this compiler warning:

Python/random.c:168:17: warning: conversion to ‘int’ from ‘long int’ may alter its value [-Wconversion]
             n = syscall(SYS_getrandom, dest, n, flags);
                 ^

But on the other hand, when I just recompiled Python with -Wconversion, I got hundreds of other warnings, so maybe there is a good reason we don’t enable that warning.
msg268702 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-16 22:02
New changeset 193f50babfa4 by Victor Stinner in branch '3.5':
py_getrandom(): use long type for the syscall() result
https://hg.python.org/cpython/rev/193f50babfa4
msg268703 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-06-16 22:04
> Adding a cast would solve this compiler warning:

I changed the code to use the long type. It should fix the warning, even if it was no more a real bug: the original bug was already fixed.

I close the issue, it's now solved.
History
Date User Action Args
2016-06-16 22:04:15vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg268703
2016-06-16 22:02:31python-devsetmessages: + msg268702
2016-06-15 00:11:10martin.pantersetmessages: + msg268595
2016-06-14 14:37:47vstinnersetmessages: + msg268560
2016-06-14 14:35:09python-devsetnosy: + python-dev
messages: + msg268559
2016-06-09 08:19:54martin.pantersetmessages: + msg267989
2016-06-09 08:16:25martin.pantersetnosy: + martin.panter
messages: + msg267986
2016-06-09 07:42:45vstinnercreate