Issue #8300 (__index__ handling in struct.pack): Remove redundant check

and improve test coverage.  Thanks Meador Inge for the patch.

diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py
index 3e7b259c969bb3c4cba909865a2bf622b12ca630..aeb52f8a286cbcd369a9a64c951e3f2aaa522fe1 100644 (file)
--- 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 fe54a47267bfeca7d54a3a02eb18480ad5da0d60..441da0315107d3adafd521ea39ac78fa5a53bada 100644 (file)
--- 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);