Hi,

I experienced a segmentation fault (when providing a wrong argument type to c_char_p) in the ctypes module under Linux and with the version 2.5 .


sundae:~$ python
Python 2.5.1c1 (release25-maint, Apr  6 2007, 22:02:36) 
[GCC 4.1.2 (Ubuntu 4.1.2-0ubuntu4)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from ctypes import *
>>> c_char_p(42)
zsh: segmentation fault (core dumped)  python
sundae:~$  


Regards,

Sebastien Martini

c_wchar_p also contains this bug.

Thanks for the bug report Sebastien.

This bug occurs in 2.5 and in the trunk, so it's not a regression.  Thomas, could you take a look?  Integers are specifically allowed here (floats are not).  The problem is when trying to print the repr:

#0  PyString_FromString (str=0x2a <Address 0x2a out of bounds>) at Objects/stringobject.c:108
#1  z_get (ptr=0x2ae1aa9d6450, size=8) at Modules/_ctypes/cfield.c:1373
#2  Simple_get_value (self=0x2ae1aa9d63f8) at Modules/_ctypes/_ctypes.c:4007
#3  Simple_repr (self=0x2ae1aa9d63f8) at Modules/_ctypes/_ctypes.c:4095
#4  PyObject_Repr (v=0x2ae1aa9d63f8) at Objects/object.c:361

This is a difficult issue.  The integer (which is interpreted as address) is allowed because some libraries use 'char *' pointers initialized to small, invalid addresses for special purposes.

On windows, printing a c_char_p(42) does not crash, it raises a ValueError instead:
>>> from ctypes import *
>>> c_char_p(42)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid string pointer 0x00B20B48
>>>

Windows does this by checking if there is a valid string at the address (see Modules/_ctypes/cfield.c, line 1366) by calling the IsBadStringPointer api function.  Do other platforms have a function that can do this check?

If not, I'm afraid we would have to give up on the very convenient repr of 'normal' c_char_p instances:

>>> c_char_p("foo bar")
c_char_p('foo bar')
>>>

and only print the address (at least on non-windows).

> Do other platforms have a function that can do this check?

There's no platform independent way to do this.

I don't know anything about ctypes, but what might be a way to handle this is providing a way to convert an integer into a pointer (like a void*).  Then all these routines could accept this void* type that you would define, but not accept a raw integer.  That means people would still have access to do the same thing they can currently do, but it would be clearer that they are playing with fire by converting any int into a pointer.

There are so many ways to use ctypes to crash, I'm not sure if this is worth fixing, except perhaps in the docs to point out the issues.

It is easy to distinguish between a c_char_p that is created from a python string from one that is created from a python int: the former has an _objects attribute set to a string:

>>> x,y = map(c_char_p, ('abc',42))
>>> (x._objects,y._objects)
('abc', None)

It may be reasonable to give up a "nice" repr for the cases where c_char_p is not known to point into a python string. You can still keep "nice" str, so users can still enjoy a crash by printing c_char_p(42). :-)

belopolsky:  _objects is only set when the c_char_p instance is created in the way you described.  It will not be set if the instance is modified by a foreign function call, so this is not robust.

nnorwitz: Sure there are many ways to crash Python with ctypes, but printing c_char_p(42) should not carsh, IMO.

I believe the best strategy would be to behave this way on Windows where the check for a valid string pointer can be made:

>>> c_char_p(42)
c_char_p(0x2A)
>>> c_char("foo")
c_char('foo')
>>>

and this way on other systems:

>>> c_char_p(42)
c_char_p(0x2A)
>>> c_char_p("foo")
c_char_p(0x2A7F3B)
>>>

The attached patch fixes this for c_char_p, a similar patch should be applied to c_wchar_p.
File Added: c_char_p.patch

theller: [_objects] will not be set if the instance is modified by a foreign function call, so this is not robust.

I thought about this before making my suggestion.  I believe people will be less surprized if objects returned from an ffi call print with funny numbers than getting c_char_p(NNNNN) after entering c_char_p("abc"). 

A few comments on the patch:

1. Since c_void_p is printed in decimal, shouldn't c_char_p be treated the same? If hexadecimal is prefered, maybe c_void_p should be changed.

2. Currently c_char_p(0) is printed as c_char_p(None), but the patch will change that to c_char_p(0).

Here's a patch that fixes the problems that belopolsky mentions: c_char_p_2.patch
File Added: c_char_p_2.patch

Committed as SVN rev 56351 (release25-maint) and rev 56352 (trunk).