(backport of r61652 and r61665 from trunk)

Issue #1471: Arguments to fcntl.ioctl are no longer broken on 64-bit OpenBSD
and similar platforms due to sign extension.

diff --git a/Lib/test/test_ioctl.py b/Lib/test/test_ioctl.py
index 2b127e2b4a95f94104ed909d0486ba8a04f3f10f..9e6d4bcfbdc2df625b02b91a8c30158d8d9b2fb7 100644 (file)
--- a/Lib/test/test_ioctl.py
+++ b/Lib/test/test_ioctl.py
@@ -14,6 +14,11 @@ try:
 except IOError:
     raise TestSkipped("Unable to open /dev/tty")
 
+try:
+    import pty
+except ImportError:
+    pty = None
+
 class IoctlTests(unittest.TestCase):
     def test_ioctl(self):
         # If this process has been put into the background, TIOCGPGRP returns
@@ -34,6 +39,30 @@ class IoctlTests(unittest.TestCase):
         self.assertEquals(r, 0)
         self.assert_(rpgrp in ids, "%s not in %s" % (rpgrp, ids))
 
+    def test_ioctl_signed_unsigned_code_param(self):
+        if not pty:
+            raise TestSkipped('pty module required')
+        mfd, sfd = pty.openpty()
+        try:
+            if termios.TIOCSWINSZ < 0:
+                set_winsz_opcode_maybe_neg = termios.TIOCSWINSZ
+                set_winsz_opcode_pos = termios.TIOCSWINSZ & 0xffffffffL
+            else:
+                set_winsz_opcode_pos = termios.TIOCSWINSZ
+                set_winsz_opcode_maybe_neg, = struct.unpack("i",
+                        struct.pack("I", termios.TIOCSWINSZ))
+
+            # We're just testing that these calls do not raise exceptions.
+            saved_winsz = fcntl.ioctl(mfd, termios.TIOCGWINSZ, "\0"*8)
+            our_winsz = struct.pack("HHHH",80,25,0,0)
+            # test both with a positive and potentially negative ioctl code
+            new_winsz = fcntl.ioctl(mfd, set_winsz_opcode_pos, our_winsz)
+            new_winsz = fcntl.ioctl(mfd, set_winsz_opcode_maybe_neg, our_winsz)
+            fcntl.ioctl(mfd, set_winsz_opcode_maybe_neg, saved_winsz)
+        finally:
+            os.close(mfd)
+            os.close(sfd)
+
 def test_main():
     run_unittest(IoctlTests)
 

diff --git a/Misc/NEWS b/Misc/NEWS
index 960f1199d0a22aa2ca21290dca29469cbca474d2..603e84711ebf7b6449cec8bff039da2ae7151798 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -143,6 +143,9 @@ Extension Modules
 
 - Issue #3120: On 64-bit Windows the subprocess module was truncating handles.
 
+- Issue #1471: Arguments to fcntl.ioctl are no longer broken on 64-bit OpenBSD
+  and similar platforms due to sign extension.
+
 Tests
 -----
 

diff --git a/Modules/fcntlmodule.c b/Modules/fcntlmodule.c
index 4e49a7d1201d8c4b36278e2bb16e16e95d9dcd01..3fa5a6013fdc41a32632e88a4dd4666272e3a380 100644 (file)
--- a/Modules/fcntlmodule.c
+++ b/Modules/fcntlmodule.c
@@ -97,11 +97,20 @@ fcntl_ioctl(PyObject *self, PyObject *args)
 {
 #define IOCTL_BUFSZ 1024
        int fd;
-       /* In PyArg_ParseTuple below, use the unsigned int 'I' format for
-          the signed int 'code' variable, because Python turns 0x8000000
-          into a large positive number (PyLong, or PyInt on 64-bit
-          platforms,) whereas C expects it to be a negative int */
-       int code;
+       /* In PyArg_ParseTuple below, we use the unsigned non-checked 'I'
+          format for the 'code' parameter because Python turns 0x8000000
+          into either a large positive number (PyLong or PyInt on 64-bit
+          platforms) or a negative number on others (32-bit PyInt)
+          whereas the system expects it to be a 32bit bit field value
+          regardless of it being passed as an int or unsigned long on
+          various platforms.  See the termios.TIOCSWINSZ constant across
+          platforms for an example of thise.
+
+          If any of the 64bit platforms ever decide to use more than 32bits
+          in their unsigned long ioctl codes this will break and need
+          special casing based on the platform being built on.
+        */
+       unsigned int code;
        int arg;
        int ret;
        char *str;