From fdaaa9c9d8a768c324a49250c91295ebce6b201a Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Sun, 4 Apr 2010 08:43:04 +0000 Subject: Issue #8300 (__index__ handling in struct.pack): Remove redundant check and improve test coverage. Thanks Meador Inge for the patch. --- Lib/test/test_struct.py | 37 +++++++++++++++++++++++++++---------- Modules/_struct.c | 7 +------ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 3e7b259..aeb52f8 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -289,6 +289,25 @@ class StructTest(unittest.TestCase): def __long__(self): return -163L + # Objects with an '__index__' method should be allowed + # to pack as integers. That is assuming the implemented + # '__index__' method returns and 'int' or 'long'. + class Indexable(object): + def __init__(self, value): + self._value = value + + def __index__(self): + return self._value + + # If the '__index__' method raises a type error, then + # '__int__' should be used with a deprecation warning. + class BadIndex(object): + def __index__(self): + raise TypeError + + def __int__(self): + return 42 + self.assertRaises((TypeError, struct.error), struct.pack, self.format, "a string") @@ -304,7 +323,7 @@ class StructTest(unittest.TestCase): # an attempt to convert a non-integer (with an # implicit conversion via __int__) should succeed, # with a DeprecationWarning - for nonint in NotAnIntNS(), NotAnIntOS(): + for nonint in NotAnIntNS(), NotAnIntOS(), BadIndex(): with check_warnings((".*integer argument expected, got non" "-integer", DeprecationWarning)) as w: got = struct.pack(self.format, nonint) @@ -315,15 +334,7 @@ class StructTest(unittest.TestCase): expected = struct.pack(self.format, int(nonint)) self.assertEqual(got, expected) - # Objects with an '__index__' method should be allowed - # to pack as integers. - class Indexable(object): - def __init__(self, value): - self._value = value - - def __index__(self): - return self._value - + # Check for legitimate values from '__index__'. for obj in (Indexable(0), Indexable(10), Indexable(17), Indexable(42), Indexable(100), Indexable(127)): try: @@ -332,6 +343,12 @@ class StructTest(unittest.TestCase): self.fail("integer code pack failed on object " "with '__index__' method") + # Check for bogus values from '__index__'. + for obj in (Indexable('a'), Indexable(u'b'), Indexable(None), + Indexable({'a': 1}), Indexable([1, 2, 3])): + self.assertRaises((TypeError, struct.error), + struct.pack, self.format, + obj) byteorders = '', '@', '=', '<', '>', '!' for code in integer_codes: diff --git a/Modules/_struct.c b/Modules/_struct.c index fe54a47..441da03 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -123,12 +123,6 @@ get_pylong(PyObject *v) w = PyNumber_Index(v); if (w != NULL) { v = w; - if (!PyInt_Check(v) && !PyLong_Check(v)) { - PyErr_SetString(PyExc_TypeError, - "__index__ method " - "returned non-integer"); - return NULL; - } /* successfully converted to an integer */ converted = 1; } @@ -175,6 +169,7 @@ get_pylong(PyObject *v) /* Ensure we own a reference to v. */ Py_INCREF(v); + assert(PyInt_Check(v) || PyLong_Check(v)); if (PyInt_Check(v)) { r = PyLong_FromLong(PyInt_AS_LONG(v)); Py_DECREF(v); -- cgit v0.12