Oren Milman added the comment:
Sorry for taking so long to come up with a patch..
------------ proposed changes ------------
1. in Python/getargs.c in seterror, the following patches:
- add a parameter 'p_format_code' - a pointer to the format code
whose conversion failed.
- replace the error message of the OverflowError with
'{fname}() argument out of range' in case all of the following are
true:
* PyErr_Occurred() and the exception is an OverflowError.
* The PyArg_* function received a format ending with a ':<fname>'
(i.e. seterror's fname argument is not NULL).
* The error occurred during a conversion to a C integer type
which might overflow, i.e. one of the format codes 'bhilLn'.
With these patches, inconsistent messages could often be fixed by merely
appending ':<fname>' to the format argument in a call to a PyArg_*
function.
Furthermore, Some inconsistent messages are actually fixed by these
patches alone.
That is because there are already many calls to PyArg_* functions with a
format ending with ':<fname>'. Some of them are followed by more
checks of the parsed arguments, which might result in raising an
OverflowError/ValueError with an inconsistent error message.
(e.g. in Modules/itertoolsmodule.c, product_new already calls
PyArg_ParseTupleAndKeywords with the format "|n:product", so with these
patches, in case we do 'itertools.product('a', repeat=1 << 1000)', the
error message would be 'product() argument out of range').
Also, ISTM this patch is helpful, regardless of this issue (#15988).
In case a PyArg_* function raises an OverflowError (because a conversion
to some C integer type failed), knowing which function called the
PyArg_* function would probably prove more helpful to a user (than
knowing which C type Python failed to convert to, and whether the
conversion failed because the number to convert was too large or too
small).
I decided to put the patch inside seterror, because (aside from its
name,) it is always called after a failure of convertitem (which is the
only caller of convertsimple).
2. in various files:
- change some OverflowError/ValueError messages for more clarity, e.g.
replace 'unsigned byte integer is greater than maximum' with
'Python int too large to convert to C unsigned char'.
- add code to "catch" OverflowError/ValueError errors, to prevent
raising inconsistent error messages
- append ':<fname>' to formats passed to PyArg_* functions, to utilize
the first proposed change (to automatically "catch"
OverflowError/ValueError errors).
3. in Lib/tests - I was already writing tests for my patches, so I guessed
I should add some of them (the basic boundary checks) to Lib/tests (in
case they didn't already exist). I ran into some issues here:
- test_pickle - I didn't manage to create an int large enough to test
my patch for save_long in Modules/_pickle.c (and so I didn't add any
test to test_pickle).
- test_stat - I didn't find a way to determine the size of mode_t on
the current platform, except for Windows (so the test I added is
incomprehensive).
4. in Objects/longobject.c, make some messages more helpful (as mentioned
in the last message I posted here).
------------ diff ------------
The proposed patches diff file is attached.
------------ tests ------------
I wrote an ugly script to test my patches, and ran it on my Windows 10 and
Ubuntu 16.04 64-bit VM. The script is attached, but it might fail on another
platform.
- Note that the script I wrote has (among others) a test for
'select.devpoll'. I couldn't run it, as I don't have a Solaris machine,
but the test is identical to that of 'select.poll', so it should run
smoothly on a Solaris machine. Theoretically.
(Each test in my script verifies the exception's error message, and not only
which exception was raised, so ISTM these kind of tests don't fit in
Lib/tests. Please let me know if you think otherwise.)
In addition, I ran 'python_d.exe -m test -j0' (on my 64-bit Windows 10 and
Ubuntu VM) with and without the patches, and got quite the same output. (That
also means my new tests in various files in Lib/tests have passed.)
##############################################################################
Note that the inconsistent messages my patches fix are only those I found
while going over each 'PyExc_OverflowError' in the codebase (in *.h and *.c
files).
However, I would probably find another bunch of inconsistent messages when I
go over each 'PyExc_ValueError' in the codebase.
I would be happy to do that, but I am afraid I won't have time to (at least)
until next June.
----------
keywords: +patch
Added file: http://bugs.python.org/file45107/issue15988_ver1.diff
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue15988>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
