[Python-checkins] r65466 - in python/branches/release25-maint: Lib/test/test_ioctl.py Misc/NEWS Modules/fcntlmodule.c

[gregory.p.smith]

Author: gregory.p.smith
Date: Mon Aug  4 02:45:34 2008
New Revision: 65466

Log:
(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.


Modified:
   python/branches/release25-maint/Lib/test/test_ioctl.py
   python/branches/release25-maint/Misc/NEWS
   python/branches/release25-maint/Modules/fcntlmodule.c

Modified: python/branches/release25-maint/Lib/test/test_ioctl.py
==============================================================================
--- python/branches/release25-maint/Lib/test/test_ioctl.py	(original)
+++ python/branches/release25-maint/Lib/test/test_ioctl.py	Mon Aug  4 02:45:34 2008
@@ -14,6 +14,11 @@
 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 @@
         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)
 

Modified: python/branches/release25-maint/Misc/NEWS
==============================================================================
--- python/branches/release25-maint/Misc/NEWS	(original)
+++ python/branches/release25-maint/Misc/NEWS	Mon Aug  4 02:45:34 2008
@@ -143,6 +143,9 @@
 
 - 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
 -----
 

Modified: python/branches/release25-maint/Modules/fcntlmodule.c
==============================================================================
--- python/branches/release25-maint/Modules/fcntlmodule.c	(original)
+++ python/branches/release25-maint/Modules/fcntlmodule.c	Mon Aug  4 02:45:34 2008
@@ -97,11 +97,20 @@
 {
 #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;