]> granicus.if.org Git - python/commitdiff
Issues #1530559, #1741130: Fix various inconsistencies in struct.pack
authorMark Dickinson <dickinsm@gmail.com>
Sun, 5 Jul 2009 10:01:24 +0000 (10:01 +0000)
committerMark Dickinson <dickinsm@gmail.com>
Sun, 5 Jul 2009 10:01:24 +0000 (10:01 +0000)
integer packing, and reenable some previously broken tests.

Lib/test/test_struct.py
Misc/NEWS
Modules/_struct.c

index ef05e3cf944cdf07a230388ed6fd7c91ad51fdec..61f48d691a2f8d5d352d9c4d4f825e619b5d1614 100644 (file)
@@ -73,7 +73,7 @@ class StructTest(unittest.TestCase):
             return
         try:
             struct.pack(format, number)
-        except (struct.error, TypeError):
+        except struct.error:
             if PY_STRUCT_FLOAT_COERCE:
                 self.fail("expected DeprecationWarning for float coerce")
         except DeprecationWarning:
@@ -220,12 +220,6 @@ class StructTest(unittest.TestCase):
 
         class IntTester(unittest.TestCase):
 
-            # XXX Most std integer modes fail to test for out-of-range.
-            # The "i" and "l" codes appear to range-check OK on 32-bit boxes, but
-            # fail to check correctly on some 64-bit ones (Tru64 Unix + Compaq C
-            # reported by Mark Favas).
-            BUGGY_RANGE_CHECK = "bBhHiIlL"
-
             def __init__(self, formatpair, bytesize):
                 super(IntTester, self).__init__(methodName='test_one')
                 self.assertEqual(len(formatpair), 2)
@@ -289,12 +283,8 @@ class StructTest(unittest.TestCase):
 
                 else:
                     # x is out of range -- verify pack realizes that.
-                    if not PY_STRUCT_RANGE_CHECKING and code in self.BUGGY_RANGE_CHECK:
-                        if verbose:
-                            print "Skipping buggy range check for code", code
-                    else:
-                        deprecated_err(pack, ">" + code, x)
-                        deprecated_err(pack, "<" + code, x)
+                    deprecated_err(pack, ">" + code, x)
+                    deprecated_err(pack, "<" + code, x)
 
                 # Much the same for unsigned.
                 code = self.unsigned_code
@@ -338,12 +328,8 @@ class StructTest(unittest.TestCase):
 
                 else:
                     # x is out of range -- verify pack realizes that.
-                    if not PY_STRUCT_RANGE_CHECKING and code in self.BUGGY_RANGE_CHECK:
-                        if verbose:
-                            print "Skipping buggy range check for code", code
-                    else:
-                        deprecated_err(pack, ">" + code, x)
-                        deprecated_err(pack, "<" + code, x)
+                    deprecated_err(pack, ">" + code, x)
+                    deprecated_err(pack, "<" + code, x)
 
             def run(self):
                 from random import randrange
@@ -374,10 +360,25 @@ class StructTest(unittest.TestCase):
                             self.test_one(x)
 
                 # Some error cases.
+                class NotAnIntNS(object):
+                    def __int__(self):
+                        return 42
+
+                    def __long__(self):
+                        return 1729L
+
+                class NotAnIntOS:
+                    def __int__(self):
+                        return 10585
+
+                    def __long__(self):
+                        return -163L
+
                 for direction in "<>":
                     for code in self.formatpair:
-                        for badobject in "a string", 3+42j, randrange:
-                            self.assertRaises((struct.error, TypeError),
+                        for badobject in ("a string", 3+42j, randrange,
+                                          NotAnIntNS(), NotAnIntOS()):
+                            self.assertRaises(struct.error,
                                                struct.pack, direction + code,
                                                badobject)
 
@@ -447,7 +448,7 @@ class StructTest(unittest.TestCase):
             import sys
             for endian in ('', '>', '<'):
                 for cls in (int, long):
-                    for fmt in ('B', 'H', 'I', 'L'):
+                    for fmt in ('B', 'H', 'I', 'L', 'Q'):
                         deprecated_err(struct.pack, endian + fmt, cls(-1))
 
                     deprecated_err(struct.pack, endian + 'B', cls(300))
@@ -455,12 +456,12 @@ class StructTest(unittest.TestCase):
 
                 deprecated_err(struct.pack, endian + 'I', sys.maxint * 4L)
                 deprecated_err(struct.pack, endian + 'L', sys.maxint * 4L)
+                deprecated_err(struct.pack, endian + 'Q', 2**64)
 
-    def XXXtest_1530559(self):
-        # XXX This is broken: see the bug report
+    def test_1530559(self):
         # SF bug 1530559. struct.pack raises TypeError where it used to convert.
         for endian in ('', '>', '<'):
-            for fmt in ('B', 'H', 'I', 'L', 'b', 'h', 'i', 'l'):
+            for fmt in ('B', 'H', 'I', 'L', 'Q', 'b', 'h', 'i', 'l', 'q'):
                 self.check_float_coerce(endian + fmt, 1.0)
                 self.check_float_coerce(endian + fmt, 1.5)
 
index 971b25e55e0450752b49c0d046c166278eb524ec..e18fe3c1501bb357770f1263d64d8d1d068c1790 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -1167,6 +1167,21 @@ C-API
 Extension Modules
 -----------------
 
+- Issues #1530559, #1741130: Fix various struct.pack inconsistencies
+  for the integer formats ('bBhHiIlLqQ').  In the following, '*'
+  represents any of '=', '<', '>'.
+
+    - Packing a float now always gives a Deprecation Warning.
+      Previously it only warned for 'I', 'L', '*B', '*H', '*I', '*L'.
+
+    - If x is not an int, long or float, then packing x will always
+      result in struct.error.  Previously an x with an __int__ method
+      could be packed by 'b', 'B', 'h', 'H', 'i', 'l', '*b', '*h'
+      ,'*i', '*l', and an x with a __long__ method could be packed by
+      'q', 'Q', '*q', '*Q'; for x with neither __int__ nor __long__,
+      TypeError used to be raised (with a confusing error message) for
+      'I', 'L', '*B', '*H', '*I', '*L', and struct.error in other cases.
+
 - Issue #4873: Fix resource leaks in error cases of pwd and grp.
 
 - Issue #4751: For hashlib algorithms provided by OpenSSL, the Python
index a7fce105a0b946a3bf52a8cd5e05c5c482ae7af3..12fec2c2a6e3225717614f634bd52a056e49f33b 100644 (file)
@@ -124,8 +124,6 @@ typedef struct { char c; _Bool x; } s_bool;
 static PyObject *
 get_pylong(PyObject *v)
 {
-       PyNumberMethods *m;
-
        assert(v != NULL);
        if (PyInt_Check(v))
                return PyLong_FromLong(PyInt_AS_LONG(v));
@@ -133,73 +131,63 @@ get_pylong(PyObject *v)
                Py_INCREF(v);
                return v;
        }
-       m = Py_TYPE(v)->tp_as_number;
-       if (m != NULL && m->nb_long != NULL) {
-               v = m->nb_long(v);
-               if (v == NULL)
+#ifdef PY_STRUCT_FLOAT_COERCE
+       if (PyFloat_Check(v)) {
+               if (PyErr_WarnEx(PyExc_DeprecationWarning, FLOAT_COERCE, 2)<0)
                        return NULL;
-               if (PyLong_Check(v))
-                       return v;
-               Py_DECREF(v);
+               return PyNumber_Long(v);
        }
+#endif
        PyErr_SetString(StructError,
                        "cannot convert argument to long");
        return NULL;
 }
 
-/* Helper routine to get a Python integer and raise the appropriate error
-   if it isn't one */
+/* Helper to convert a Python object to a C long.  Raise StructError and
+   return -1 if v has the wrong type or is outside the range of a long. */
 
 static int
 get_long(PyObject *v, long *p)
 {
-       long x = PyInt_AsLong(v);
-       if (x == -1 && PyErr_Occurred()) {
-#ifdef PY_STRUCT_FLOAT_COERCE
-               if (PyFloat_Check(v)) {
-                       PyObject *o;
-                       int res;
-                       PyErr_Clear();
-                       if (PyErr_WarnEx(PyExc_DeprecationWarning, FLOAT_COERCE, 2) < 0)
-                               return -1;
-                       o = PyNumber_Int(v);
-                       if (o == NULL)
-                               return -1;
-                       res = get_long(o, p);
-                       Py_DECREF(o);
-                       return res;
-               }
-#endif
-               if (PyErr_ExceptionMatches(PyExc_TypeError))
+       long x;
+
+       v = get_pylong(v);
+       if (v == NULL)
+               return -1;
+       assert(PyLong_Check(v));
+       x = PyLong_AsLong(v);
+       Py_DECREF(v);
+       if (x == (long)-1 && PyErr_Occurred()) {
+               if (PyErr_ExceptionMatches(PyExc_OverflowError))
                        PyErr_SetString(StructError,
-                                       "required argument is not an integer");
+                                       "argument out of range");
                return -1;
        }
        *p = x;
        return 0;
 }
 
-
 /* Same, but handling unsigned long */
 
 #ifndef PY_STRUCT_OVERFLOW_MASKING
 static int
 get_ulong(PyObject *v, unsigned long *p)
 {
-       if (PyLong_Check(v)) {
-               unsigned long x = PyLong_AsUnsignedLong(v);
-               if (x == (unsigned long)(-1) && PyErr_Occurred())
-                       return -1;
-               *p = x;
-               return 0;
-       }
-       if (get_long(v, (long *)p) < 0)
+       unsigned long x;
+
+       v = get_pylong(v);
+       if (v == NULL)
                return -1;
-       if (((long)*p) < 0) {
-               PyErr_SetString(StructError,
-                               "unsigned argument is < 0");
+       assert(PyLong_Check(v));
+       x = PyLong_AsUnsignedLong(v);
+       Py_DECREF(v);
+       if (x == (unsigned long)-1 && PyErr_Occurred()) {
+               if (PyErr_ExceptionMatches(PyExc_OverflowError))
+                       PyErr_SetString(StructError,
+                                       "argument out of range");
                return -1;
        }
+       *p = x;
        return 0;
 }
 #endif  /* PY_STRUCT_OVERFLOW_MASKING */
@@ -219,8 +207,12 @@ get_longlong(PyObject *v, PY_LONG_LONG *p)
        assert(PyLong_Check(v));
        x = PyLong_AsLongLong(v);
        Py_DECREF(v);
-       if (x == (PY_LONG_LONG)-1 && PyErr_Occurred())
+       if (x == (PY_LONG_LONG)-1 && PyErr_Occurred()) {
+               if (PyErr_ExceptionMatches(PyExc_OverflowError))
+                       PyErr_SetString(StructError,
+                                       "argument out of range");
                return -1;
+       }
        *p = x;
        return 0;
 }
@@ -238,8 +230,12 @@ get_ulonglong(PyObject *v, unsigned PY_LONG_LONG *p)
        assert(PyLong_Check(v));
        x = PyLong_AsUnsignedLongLong(v);
        Py_DECREF(v);
-       if (x == (unsigned PY_LONG_LONG)-1 && PyErr_Occurred())
+       if (x == (unsigned PY_LONG_LONG)-1 && PyErr_Occurred()) {
+               if (PyErr_ExceptionMatches(PyExc_OverflowError))
+                       PyErr_SetString(StructError,
+                                       "argument out of range");
                return -1;
+       }
        *p = x;
        return 0;
 }
@@ -256,79 +252,81 @@ get_ulonglong(PyObject *v, unsigned PY_LONG_LONG *p)
 static int
 get_wrapped_long(PyObject *v, long *p)
 {
-       if (get_long(v, p) < 0) {
-               if (PyLong_Check(v) &&
-                   PyErr_ExceptionMatches(PyExc_OverflowError)) {
-                       PyObject *wrapped;
-                       long x;
-                       PyErr_Clear();
-#ifdef PY_STRUCT_FLOAT_COERCE
-                       if (PyFloat_Check(v)) {
-                               PyObject *o;
-                               int res;
-                               PyErr_Clear();
-                               if (PyErr_WarnEx(PyExc_DeprecationWarning, FLOAT_COERCE, 2) < 0)
-                                       return -1;
-                               o = PyNumber_Int(v);
-                               if (o == NULL)
-                                       return -1;
-                               res = get_wrapped_long(o, p);
-                               Py_DECREF(o);
-                               return res;
-                       }
-#endif
-                       if (PyErr_WarnEx(PyExc_DeprecationWarning, INT_OVERFLOW, 2) < 0)
-                               return -1;
-                       wrapped = PyNumber_And(v, pylong_ulong_mask);
-                       if (wrapped == NULL)
-                               return -1;
-                       x = (long)PyLong_AsUnsignedLong(wrapped);
-                       Py_DECREF(wrapped);
-                       if (x == -1 && PyErr_Occurred())
-                               return -1;
-                       *p = x;
-               } else {
-                       return -1;
-               }
+       PyObject *wrapped;
+       long x;
+
+       v = get_pylong(v);
+       if (v == NULL)
+               return -1;
+       assert(PyLong_Check(v));
+
+       x = PyLong_AsLong(v);
+       if (!(x == (long)-1 && PyErr_Occurred())) {
+               /* PyLong_AsLong succeeded; no need to wrap */
+               Py_DECREF(v);
+               *p = x;
+               return 0;
+       }
+       if (!PyErr_ExceptionMatches(PyExc_OverflowError)) {
+               Py_DECREF(v);
+               return -1;
+       }
+
+       PyErr_Clear();
+       if (PyErr_WarnEx(PyExc_DeprecationWarning, INT_OVERFLOW, 2) < 0) {
+               Py_DECREF(v);
+               return -1;
        }
+       wrapped = PyNumber_And(v, pylong_ulong_mask);
+       Py_DECREF(v);
+       if (wrapped == NULL)
+               return -1;
+       /* XXX we're relying on the (long) cast to preserve the value modulo
+          ULONG_MAX+1, but the C standards don't guarantee this */
+       x = (long)PyLong_AsUnsignedLong(wrapped);
+       Py_DECREF(wrapped);
+       if (x == (long)-1 && PyErr_Occurred())
+               return -1;
+       *p = x;
        return 0;
 }
 
 static int
 get_wrapped_ulong(PyObject *v, unsigned long *p)
 {
-       long x = (long)PyLong_AsUnsignedLong(v);
-       if (x == -1 && PyErr_Occurred()) {
-               PyObject *wrapped;
-               PyErr_Clear();
-#ifdef PY_STRUCT_FLOAT_COERCE
-               if (PyFloat_Check(v)) {
-                       PyObject *o;
-                       int res;
-                       PyErr_Clear();
-                       if (PyErr_WarnEx(PyExc_DeprecationWarning, FLOAT_COERCE, 2) < 0)
-                               return -1;
-                       o = PyNumber_Int(v);
-                       if (o == NULL)
-                               return -1;
-                       res = get_wrapped_ulong(o, p);
-                       Py_DECREF(o);
-                       return res;
-               }
-#endif
-               wrapped = PyNumber_And(v, pylong_ulong_mask);
-               if (wrapped == NULL)
-                       return -1;
-               if (PyErr_WarnEx(PyExc_DeprecationWarning, INT_OVERFLOW, 2) < 0) {
-                       Py_DECREF(wrapped);
-                       return -1;
-               }
-               x = (long)PyLong_AsUnsignedLong(wrapped);
-               Py_DECREF(wrapped);
-               if (x == -1 && PyErr_Occurred())
-                       return -1;
+       PyObject *wrapped;
+       unsigned long x;
+
+       v = get_pylong(v);
+       if (v == NULL)
+               return -1;
+       assert(PyLong_Check(v));
+
+       x = PyLong_AsUnsignedLong(v);
+       if (!(x == (unsigned long)-1 && PyErr_Occurred())) {
+               Py_DECREF(v);
+               *p = x;
+               return 0;
        }
-       *p = (unsigned long)x;
+       if (!PyErr_ExceptionMatches(PyExc_OverflowError)) {
+               Py_DECREF(v);
+               return -1;
+       }
+
+       PyErr_Clear();
+       if (PyErr_WarnEx(PyExc_DeprecationWarning, INT_OVERFLOW, 2) < 0) {
+               Py_DECREF(v);
+               return -1;
+       }
+       wrapped = PyNumber_And(v, pylong_ulong_mask);
+       Py_DECREF(v);
+       if (wrapped == NULL)
+               return -1;
+       x = PyLong_AsUnsignedLong(wrapped);
+       Py_DECREF(wrapped);
+       if (x == (unsigned long)-1 && PyErr_Occurred())
+               return -1;
+       *p = x;
        return 0;
 }