classification
Title: ioctl request argument broken on 64-bit OpenBSD or OS X
Type: behavior Stage:
Components: Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: fbvortex, georg.brandl, gregory.p.smith, jafo, loewis, msavory, nicm
Priority: normal Keywords: 64bit, easy, patch

Created on 2007-11-20 00:27 by fbvortex, last changed 2008-08-04 00:46 by gregory.p.smith. This issue is now closed.

Messages (15)
msg57673 - (view) Author: (fbvortex) Date: 2007-11-20 00:27
The following lines of code work on Linux platforms (amd64), and 32-bit
OpenBSD (i386), but not on 64-bit OpenBSD platforms (at least not on
amd64 or sparc64):

import fcntl,os,pty,termios,select,sys,struct,pwd,signal,os
pid,fd=pty.fork()
fcntl.ioctl(fd, termios.TIOCSWINSZ, struct.pack("HHHH",80,25,0,0))

This gives an "IOError: [Errno 25] Inappropriate ioctl for device"
exception.

The python version in question on OpenBSD is:

Python 2.5.1 (r251:54863, Nov 16 2007, 18:16:44) 
[GCC 3.3.5 (propolice)] on openbsd4

The winsize structure as dumped using sizeof(struct winsize) under
OpenBSD/sparc64 is 8 bytes large, and so is the result from the
struct.pack operation.  Forcing the endianness big or little in the
struct.pack formatting string has no effect on the error.
msg57676 - (view) Author: (fbvortex) Date: 2007-11-20 00:50
The following C code, when compiled with -lutil runs without reporting
any errors on both the sparc64 and i386 platforms on OpenBSD:

#include <util.h>
#include <utmp.h>
#include <termios.h>
#include <sys/ioctl.h>
#include <stdio.h>
 
int main(void)
{
        int fd;
        struct winsize w;
 
        w.ws_row = 25;
        w.ws_col = 80;
        w.ws_xpixel = w.ws_ypixel = 0;
 
        forkpty(&fd, NULL, NULL, NULL);
        if (ioctl(fd,TIOCSWINSZ, &w) == -1)
                perror("ioctl");
        return 0;
}
msg57686 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2007-11-20 02:58
So what's the definition of struct winsize on these systems?

Also, why do you think this is a bug in Python? AFAICT, the specific
ioctl call does not occur in Python, but in your own code.
msg57697 - (view) Author: Nicholas Marriott (nicm) Date: 2007-11-20 09:25
I can also reproduce this on OpenBSD/amd64 and was one of the people who
discussed it with the submitter before he created this report.

> So what's the definition of struct winsize on these systems?

The definition of struct winsize on both 32-bit and 64-bit OpenBSD
platforms is:

struct winsize {
        unsigned short  ws_row;         /* rows, in characters */
        unsigned short  ws_col;         /* columns, in characters */
        unsigned short  ws_xpixel;      /* horizontal size, pixels */
        unsigned short  ws_ypixel;      /* vertical size, pixels */
};

We have verified that (sizeof (struct winsize)) is 8 on all three
platforms (32-bit i386, and 64-bit amd64 and sparc64).

> Also, why do you think this is a bug in Python? AFAICT, the specific 
> ioctl call does not occur in Python, but in your own code.

This Python code fails:

fcntl.ioctl(fd, termios.TIOCSWINSZ, struct.pack("HHHH",80,25,0,0))

(Error: IOError: [Errno 25] Inappropriate ioctl for device) This code
also fails with the same error message (I would expect EINVAL instead):

fcntl.ioctl(0, termios.TIOCSWINSZ, "")

Corresponding test C code to call the ioctl (as listed in a previous
message) works without problems on all three platforms. I don't know how
Python fcntl.ioctl works and perhaps the problem is not Python, but I am
having to try fairly hard to think what else could be involved.

The constant appears to be the correct ioctl number. (Python code:

python2.5 -c 'import termios; print "%d" % (termios.TIOCSWINSZ)'

matches C code:

#include <termios.h>
int main(void) { printf("%ld\n", TIOCSWINSZ); }

on all three platforms.)

I am told it works on Linux. However, Linux declares ioctl as:

int ioctl(int d, int request, ...); 

So 'request' is 32-bits, but on OpenBSD ioctl is declared as:

int ioctl(int d, unsigned long request, ...);

So on amd64 and sparc64, 'request' is 64-bits. Could this be a factor?

-- Nicholas.
msg57699 - (view) Author: Nicholas Marriott (nicm) Date: 2007-11-20 10:08
Okay, looks like my guess was correct. The diff at the end of this mail 
makes it work on OpenBSD/amd64 and it continues to work on i386, but it 
will probably break Linux (due to the problem it was working around 
mentioned in the comment at the beginning). I don't know enough about 
the Python build system to suggest a correct fix. (SUSv3 seems to 
suggest ioctl's request should be int, so perhaps OpenBSD is ultimately 
at fault.)

fcntl_ioctl in fcntlmodule.c seems to have some other potential 
problems on 64-bit archs too, as it seems to coerce the ioctl argument 
(potentially a pointer) into an int.

-- Nicholas.

--- fcntlmodule.c.orig	Tue Nov 20 09:39:12 2007
+++ fcntlmodule.c	Tue Nov 20 09:59:50 2007
@@ -101,7 +101,7 @@
 	   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;
+	unsigned long code;
 	int arg;
 	int ret;
 	char *str;
@@ -109,7 +109,7 @@
 	int mutate_arg = 1;
  	char buf[IOCTL_BUFSZ+1];  /* argument plus NUL byte */
 
-	if (PyArg_ParseTuple(args, "O&Iw#|i:ioctl",
+	if (PyArg_ParseTuple(args, "O&Lw#|i:ioctl",
                              conv_descriptor, &fd, &code, 
 			     &str, &len, &mutate_arg)) {
 		char *arg;
@@ -160,7 +160,7 @@
 	}
 
 	PyErr_Clear();
-	if (PyArg_ParseTuple(args, "O&Is#:ioctl",
+	if (PyArg_ParseTuple(args, "O&Ls#:ioctl",
                              conv_descriptor, &fd, &code, &str, &len)) 
{
 		if (len > IOCTL_BUFSZ) {
 			PyErr_SetString(PyExc_ValueError,
@@ -182,7 +182,7 @@
 	PyErr_Clear();
 	arg = 0;
 	if (!PyArg_ParseTuple(args,
-	     "O&I|i;ioctl requires a file or file descriptor,"
+	     "O&L|i;ioctl requires a file or file descriptor,"
 	     " an integer and optionally an integer or buffer argument",
 			      conv_descriptor, &fd, &code, &arg)) {
 	  return NULL;
msg58237 - (view) Author: Mike Savory (msavory) Date: 2007-12-06 03:46
Similar issue seen in 64 bit OSX 10.5

  File "./ajaxterm.py", line 418, in create
    fcntl.ioctl(fd, 
struct.unpack('i',struct.pack('I',termios.TIOCSWINSZ))[0], 
struct.pack("HHHH",h,w,0,0))
IOError: [Errno 25] Inappropriate ioctl for device

$ python
Python 2.5.1 (r251:54863, Oct  5 2007, 21:08:09) 
[GCC 4.0.1 (Apple Inc. build 5465)] on darwin
msg63803 - (view) Author: Sean Reifschneider (jafo) * (Python committer) Date: 2008-03-17 23:05
I think this proposed change needs some research into what the standards
say about ioctl's return code, and if the change to a long return code
is done then test it on Linux to see if it breaks things or if it works.
 Thoughts?
msg63897 - (view) Author: Nicholas Marriott (nicm) Date: 2008-03-18 06:14
It's not the return value, it's the request argument (second argument).
I thought SUSv3 had it as an int, but looking again its ioctl seems to
be a) optional and b) all about STREAMS, so I don't think it applies:

http://www.opengroup.org/onlinepubs/009695399/functions/ioctl.html

-- Nicholas
msg63898 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-03-18 06:34
i'd say the patch is fine.

on linux ioctl takes an int.
on openbsd it takes an unsigned long.
on something else it might even take its own type like an ioctl_t.

regardless, treating the parameter as either a long or unsigned long
will work properly as that will cast downwards to the actual used
parameter type correctly in the C code.
msg64124 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-03-19 23:44
I am unable to reproduce this problem at all on Mac OS X 10.4 or 10.5
with 32-bit or 64-bit python trunk builds.

I have however checked in a fix that'll prevent negative code values
passed in from being sign extended to 64-bits when converted to an
unsigned long on platforms that use those.

A unittest for fcntl.ioctl behavior when passed both positive and
negative values is included.

trunk r61652

while the Linux man page describe's ioctl's code parameter as an int,
the sys/ioctl.h file on linux has it as an unsigned long int in the
function prototype.  So its quite possible it is actually an unsigned
long on all platforms and we should change unsigned int code to unsigned
long code and the 'I' to a 'k'.

However if that is done we -might- have a sign extension problem again
if anyone is passing a negative value in and expecting it to act like a
32-bit value.   If the OS kernels only look at the lower 32-bits of the
ioctl code during the syscall it'd work, but if it tested the 64bit
value -123 64bit != -123 32bit.  The better answer to that would be to
just reject all negative values and fix any defined ioctl code constants
to be positive on all platforms.

fbvortex.  Can you apply the change from svn to your python and test
that it works properly on your 64-bit OpenBSD system?  Thanks.

paste a note here once you have with the results.
msg64155 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-03-20 05:44
r61665 moves the test to test_ioctl instead of test_fcntl.  it also
forces it to only run when a pty is present rather than assuming fd 0
msg70078 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-07-20 11:22
Something else to do here?
msg70088 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-07-20 18:08
i believe this is fixed by the two changes mentioned above, i was
waiting for fbvortex to confirm the fix on his 64-bit OpenBSD system.

these changes need backporting to release25-maint.
msg70089 - (view) Author: Nicholas Marriott (nicm) Date: 2008-07-20 19:23
I was going to test this on sparc64 (I no longer have access to amd64)
but I've been busy/on holiday, I'll try to do it this week.
msg70673 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2008-08-04 00:46
committed to release25-maint as r65466.  if testing on 64bit openbsd or
similar reveals that this is not fixed, please reopen the bug.
History
Date User Action Args
2008-08-04 00:46:44gregory.p.smithsetstatus: open -> closed
messages: + msg70673
2008-07-20 19:23:42nicmsetmessages: + msg70089
2008-07-20 18:08:20gregory.p.smithsetstatus: pending -> open
messages: + msg70088
versions: - Python 2.6, Python 3.0
2008-07-20 11:22:00georg.brandlsetnosy: + georg.brandl
messages: + msg70078
2008-03-20 05:44:28gregory.p.smithsetmessages: + msg64155
2008-03-19 23:44:31gregory.p.smithsetstatus: open -> pending
resolution: fixed
messages: + msg64124
2008-03-18 06:35:52gregory.p.smithsetkeywords: + 64bit
title: ioctl doesn't work properly on 64-bit OpenBSD -> ioctl request argument broken on 64-bit OpenBSD or OS X
2008-03-18 06:34:31gregory.p.smithsetstatus: pending -> open
versions: + Python 2.6, Python 3.0
nosy: + gregory.p.smith
messages: + msg63898
assignee: gregory.p.smith
keywords: + patch, easy
resolution: not a bug -> (no value)
2008-03-18 06:14:54nicmsetmessages: + msg63897
2008-03-17 23:05:11jafosetpriority: normal
nosy: + jafo
messages: + msg63803
2007-12-06 03:46:46msavorysetnosy: + msavory
messages: + msg58237
2007-11-20 10:08:31nicmsetmessages: + msg57699
2007-11-20 09:25:19nicmsetnosy: + nicm
messages: + msg57697
2007-11-20 02:58:25loewissetstatus: open -> pending
resolution: not a bug
messages: + msg57686
nosy: + loewis
2007-11-20 00:50:01fbvortexsetmessages: + msg57676
2007-11-20 00:27:07fbvortexcreate