Issues with getargs_n() and PyNumber_Index.

[tpn]

BPO	2440
Nosy	@loewis, @benjaminp, @tpn
Files	getargs_and_abstract.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/tpn'
closed_at = <Date 2008-04-10.16:27:41.002>
created_at = <Date 2008-03-20.22:00:42.041>
labels = ['interpreter-core']
title = 'Issues with getargs_n() and PyNumber_Index.'
updated_at = <Date 2008-04-22.19:03:23.829>
user = 'https://github.com/tpn'

bugs.python.org fields:

activity = <Date 2008-04-22.19:03:23.829>
actor = 'trent'
assignee = 'trent'
closed = True
closed_date = <Date 2008-04-10.16:27:41.002>
closer = 'trent'
components = ['Interpreter Core']
creation = <Date 2008-03-20.22:00:42.041>
creator = 'trent'
dependencies = []
files = ['9795']
hgrepos = []
issue_num = 2440
keywords = ['patch', '64bit']
message_count = 7.0
messages = ['64212', '64214', '65295', '65325', '65326', '65327', '65678']
nosy_count = 3.0
nosy_names = ['loewis', 'benjamin.peterson', 'trent']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue2440'
versions = ['Python 3.0']

[tpn]

test_getargs2 fails on Win x64:

test_getargs2 is failing on Windows x64:
test test_getargs2 failed -- Traceback (most recent call last):
  File
"S:\buildbots\python.x64\3.0.nelson-win64\build\lib\test\test_getargs2.py",
line 190, in test_n
    self.failUnlessEqual(99, getargs_n(Long()))
TypeError: 'Long' object cannot be interpreted as an integer

The problem is twofold: case 'n' on Win x64 (where SIZEOF_SIZE_T !=
SIZEOF_LONG) had a broken code path and needed updating. Also, the
fallback to 'l' for systems where SIZEOF_SIZE_T == SIZEOF_LONG wasn't
correct -- it should still do a PyNumber_Index() check, and then use
PyLong_AsSize_t() to extract the value.

The attached patch corrects the behaviour on 32-bit and 64-bit systems,
including Windows. However, it has now uncovered another bug in Windows
x64:

>>> from _testcapi import getargs_n
>>> getargs_n(sys.maxsize)
9223372036854775807
>>> getargs_n(-sys.maxsize)
1
>>> getargs_n(-sys.maxsize-1)
0

After a bit of investigation with Martin, the logic in PyLong_AsSize_t()
is incorrect and needs to be reworked to handle negative maximums properly.

[tpn]

Attach a slightly cleaner patch, take 2.

[tpn]

Committed patch in r62269. I'll raise a separate tracker issue for
PyLong_AsSsize_t as it isn't related to this issue.

[benjaminp]

Trent, that commit broke several tests (test_builtin for one) that
relied on a floats not being interpreted as longs.

[tpn]

Eek, so it does, thanks. Applied the following patch on a bunch of 32-
bit/x64 systems, testing now, so far everything looks good...

Index: abstract.c
===================================================================

--- abstract.c  (revision 62279)
+++ abstract.c  (working copy)
@@ -1240,7 +1240,7 @@
                        return NULL;
                }
        }
-       else if (m && m->nb_int != NULL) {
+       else if (m && m->nb_int != NULL && m->nb_float == NULL) {
                result = m->nb_int(item);
                if (result && !PyLong_Check(result)) {
                        PyErr_Format(PyExc_TypeError,

[benjaminp]

Thanks for fixing that.

[tpn]

Update: the changes committed on r62269 and r62279 were incorrect and
reverted in r62292.

Log:
bpo-2440: revert r62269 and r62279. These changes were made in an
effort to fix test_args2.Signed_TestCase.test_n(), which was failing on
Windows x64 on the following line: 'self.failUnlessEqual(99, getargs_n
(Long()))'. Although the two commits *did* fix the test on Windows
x64, it's become clear that it's the test that's incorrect, and the
changes to PyNumber_Index() in particular were not warranted (and
actually violate PEP-357). This commit will get us back to where we
were at r62268, before I started butchering things.

The reworked patch fixes test_getargs2.py, such that it verifies Long()
and Int() can't be used as indexes. It also fixes the handling of 'n'
in getargs.c's convertsimple(). Committed in r62462.