Fix bug

[ 555817 ] Flawed fcntl.ioctl implementation.

with my patch that allows for an array to be mutated when passed
as the buffer argument to ioctl() (details complicated by
backwards compatibility considerations -- read the docs!).

4 files changed, 171 insertions, 18 deletions

diff --git a/Doc/lib/libfcntl.tex b/Doc/lib/libfcntl.tex
index 645a97e..6eccb4a 100644
--- a/Doc/lib/libfcntl.tex
+++ b/Doc/lib/libfcntl.tex
@@ -47,10 +47,57 @@ The module defines the following functions:
   raised.
 \end{funcdesc}
 
-\begin{funcdesc}{ioctl}{fd, op, arg}
-  This function is identical to the \function{fcntl()} function, except
-  that the operations are typically defined in the library module
-  \refmodule{termios}.
+\begin{funcdesc}{ioctl}{fd, op\optional{, arg\optional{, mutate_flag}}}
+  This function is identical to the \function{fcntl()} function,
+  except that the operations are typically defined in the library
+  module \refmodule{termios} and the argument handling is even more
+  complicated.
+  
+  The parameter \var{arg} can be one of an integer, absent (treated
+  identically to the integer \code{0}), an object supporting the
+  read-only buffer interface (most likely a plain Python string) or an
+  object supporting the read-write buffer interface.
+  
+  In all but the last case, behaviour is as for the \function{fcntl()}
+  function.
+  
+  If a mutable buffer is passed, then the behaviour is determined by
+  the value of the \var{mutate_flag} parameter.
+  
+  If it is false, the buffer's mutability is ignored and behaviour is
+  as for a read-only buffer, except that the 1024 byte limit mentioned
+  above is avoided -- so long as the buffer you pass is longer than
+  what the operating system wants to put there, things should work.
+  
+  If \var{mutate_flag} is true, then the buffer is (in effect) passed
+  to the underlying \function{ioctl()} system call, the latter's
+  return code is passed back to the calling Python, and the buffer's
+  new contents reflect the action of the \function{ioctl}.  This is a
+  slight simplification, because if the supplied buffer is less than
+  1024 bytes long it is first copied into a static buffer 1024 bytes
+  long which is then passed to \function{ioctl} and copied back into
+  the supplied buffer.
+  
+  If \var{mutate_flag} is not supplied, then in 2.3 it defaults to
+  false.  This is planned to change over the next few Python versions:
+  in 2.4 failing to supply \var{mutate_flag} will get a warning but
+  the same behavior and in versions later than 2.5 it will default to
+  true.
+
+  An example:
+
+\begin{verbatim}
+>>> import array, fnctl, struct, termios, os
+>>> os.getpgrp()
+13341
+>>> struct.unpack('h', fcntl.ioctl(0, termios.TIOCGPGRP, "  "))[0]
+13341
+>>> buf = array.array('h', [0])
+>>> fcntl.ioctl(0, termios.TIOCGPGRP, buf, 1)
+0
+>>> buf
+array('h', [13341])
+\end{verbatim}
 \end{funcdesc}
 
 \begin{funcdesc}{flock}{fd, op}
@@ -122,7 +169,7 @@ better.
 \begin{seealso}
   \seemodule{os}{The \function{os.open} function supports locking flags
                  and is available on a wider variety of platforms than 
-		 the \function{fcntl.lockf} and \function{fcntl.flock}
-		 functions, providing a more platform-independent file
-		 locking facility.}
+                 the \function{fcntl.lockf} and \function{fcntl.flock}
+                 functions, providing a more platform-independent file
+                 locking facility.}
 \end{seealso}
diff --git a/Lib/test/test_ioctl.py b/Lib/test/test_ioctl.py
new file mode 100644
index 0000000..a6a59c5
--- /dev/null
+++ b/Lib/test/test_ioctl.py
@@ -0,0 +1,31 @@
+import unittest
+from test_support import TestSkipped, run_unittest
+import os, struct
+try:
+    import fcntl, termios
+except ImportError:
+    raise TestSkipped("No fcntl or termios module")
+if not hasattr(termios,'TIOCGPGRP'):
+    raise TestSkipped("termios module doesn't have TIOCGPGRP")
+
+class IoctlTests(unittest.TestCase):
+    def test_ioctl(self):
+        pgrp = os.getpgrp()
+        tty = open("/dev/tty", "r")
+        r = fcntl.ioctl(tty, termios.TIOCGPGRP, "    ")
+        self.assertEquals(pgrp, struct.unpack("i", r)[0])
+
+    def test_ioctl_mutate(self):
+        import array
+        buf = array.array('i', [0])
+        pgrp = os.getpgrp()
+        tty = open("/dev/tty", "r")
+        r = fcntl.ioctl(tty, termios.TIOCGPGRP, buf, 1)
+        self.assertEquals(r, 0)
+        self.assertEquals(pgrp, buf[0])
+
+def test_main():
+    run_unittest(IoctlTests)
+
+if __name__ == "__main__":
+    test_main()
diff --git a/Misc/NEWS b/Misc/NEWS
index a6dc7c6..bb13310 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -24,6 +24,9 @@ Core and builtins
 Extension modules
 -----------------
 
+- Modified the fcntl.ioctl() function to allow modification of a passed
+  mutable buffer (for details see the reference documentation).
+
 - Made user requested changes to the itertools module.
   Subsumed the times() function into repeat().
   Added chain() and cycle().
diff --git a/Modules/fcntlmodule.c b/Modules/fcntlmodule.c
index c5d9b4d..c495a77 100644
--- a/Modules/fcntlmodule.c
+++ b/Modules/fcntlmodule.c
@@ -99,8 +99,62 @@ fcntl_ioctl(PyObject *self, PyObject *args)
 	int ret;
 	char *str;
 	int len;
+	int mutate_arg = 0;
 	char buf[1024];
 
+	if (PyArg_ParseTuple(args, "O&iw#|i:ioctl",
+                             conv_descriptor, &fd, &code, 
+			     &str, &len, &mutate_arg)) {
+		char *arg;
+
+		if (PyTuple_Size(args) == 3) {
+			/* warning goes here in 2.4 */
+			mutate_arg = 0;
+		}
+	       	if (mutate_arg) {
+			if (len <= sizeof buf) {
+				memcpy(buf, str, len);
+				arg = buf;
+			} 
+			else {
+				arg = str;
+			}
+		}
+		else {
+			if (len > sizeof buf) {
+				PyErr_SetString(PyExc_ValueError,
+					"ioctl string arg too long");
+				return NULL;
+			}
+			else {
+				memcpy(buf, str, len);
+				arg = buf;
+			}
+		}
+		if (buf == arg) {
+			Py_BEGIN_ALLOW_THREADS /* think array.resize() */
+			ret = ioctl(fd, code, arg);
+			Py_END_ALLOW_THREADS
+		}
+		else {
+			ret = ioctl(fd, code, arg);
+		}
+		if (mutate_arg && (len < sizeof buf)) {
+			memcpy(str, buf, len);
+		}
+		if (ret < 0) {
+			PyErr_SetFromErrno(PyExc_IOError);
+			return NULL;
+		}
+		if (mutate_arg) {
+			return PyInt_FromLong(ret);
+		}
+		else {
+			return PyString_FromStringAndSize(buf, len);
+		}
+	}
+
+	PyErr_Clear();
 	if (PyArg_ParseTuple(args, "O&is#:ioctl",
                              conv_descriptor, &fd, &code, &str, &len)) {
 		if (len > sizeof buf) {
@@ -123,7 +177,7 @@ fcntl_ioctl(PyObject *self, PyObject *args)
 	arg = 0;
 	if (!PyArg_ParseTuple(args,
 	     "O&i|i;ioctl requires a file or file descriptor,"
-	     " an integer and optionally a third integer or a string",
+	     " an integer and optionally a integer or buffer argument",
 			      conv_descriptor, &fd, &code, &arg)) {
 	  return NULL;
 	}
@@ -138,17 +192,35 @@ fcntl_ioctl(PyObject *self, PyObject *args)
 }
 
 PyDoc_STRVAR(ioctl_doc,
-"ioctl(fd, opt, [arg])\n\
+"ioctl(fd, opt[, arg[, mutate_flag]])\n\
 \n\
-Perform the requested operation on file descriptor fd.  The operation\n\
-is defined by op and is operating system dependent.  Typically these\n\
-codes can be retrieved from the library module IOCTL.  The argument arg\n\
-is optional, and defaults to 0; it may be an int or a string. If arg is\n\
-given as a string, the return value of ioctl is a string of that length,\n\
-containing the resulting value put in the arg buffer by the operating system.\n\
-The length of the arg string is not allowed to exceed 1024 bytes. If the arg\n\
-given is an integer or if none is specified, the result value is an integer\n\
-corresponding to the return value of the ioctl call in the C code.");
+Perform the requested operation on file descriptor fd.  The operation is\n\
+defined by op and is operating system dependent.  Typically these codes are\n\
+retrieved from the fcntl or termios library modules.\n\
+\n\
+The argument arg is optional, and defaults to 0; it may be an int or a\n\
+buffer containing character data (most likely a string or an array). \n\
+\n\
+If the argument is a mutable buffer (such as an array) and if the\n\
+mutate_flag argument (which is only allowed in this case) is true then the\n\
+buffer is (in effect) passed to the operating system and changes made by\n\
+the OS will be reflected in the contents of the buffer after the call has\n\
+returned.  The return value is the integer returned by the ioctl system\n\
+call.\n\
+\n\
+If the argument is a mutable buffer and the mutable_flag argument is not\n\
+passed or is false, the behavior is as if a string had been passed.  This\n\
+behavior will change in future releases of Python.\n\
+\n\
+If the argument is an immutable buffer (most likely a string) then a copy\n\
+of the buffer is passed to the operating system and the return value is a\n\
+string of the same length containing whatever the operating system put in\n\
+the buffer.  The length of the arg buffer in this case is not allowed to\n\
+exceed 1024 bytes.\n\
+\n\
+If the arg given is an integer or if none is specified, the result value is\n\
+an integer corresponding to the return value of the ioctl call in the C\n\
+code.");
 
 
 /* flock(fd, operation) */