classification
Title: Resize doesn't change reported length on create_string_buffer()
Type: enhancement Stage: resolved
Components: ctypes Versions: Python 2.7
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Dustin.Oprea, beng94, eryksun, martin.panter, meador.inge
Priority: normal Keywords: patch

Created on 2014-04-22 14:02 by Dustin.Oprea, last changed 2016-02-05 08:48 by eryksun. This issue is now closed.

Files
File name Uploaded Description Edit
resize.patch beng94, 2016-02-04 23:49 review
Messages (8)
msg217005 - (view) Author: Dustin Oprea (Dustin.Oprea) Date: 2014-04-22 14:02
The memory is resized, but the value returned by len() doesn't change:

>>> b = ctypes.create_string_buffer(23)
>>> len(b)
23
>>> b.raw = '0' * 23
>>> b.raw = '0' * 24
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: string too long

>>> ctypes.resize(b, 28)
>>> len(b)
23
>>> b.raw = '0' * 28
>>> b.raw = '0' * 29
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: string too long
msg259575 - (view) Author: Tamás Bence Gedai (beng94) * Date: 2016-02-04 16:38
I found out that if you modify Modules/_cpython/callproc.c resize function in a way that you set the obj->b_length to size, the len function returns the correct value.

To be able to provide a proper patch, I'd like to look into len's implementation, can someone tell me where to look for it?
msg259600 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-04 22:45
I’m not sure if resize() should change the len(). Dustin, why do you think it should? Not all ctypes objects even implement len(). The len() of 10 seems embedded in the class of the return value:

>>> b
<ctypes.c_char_Array_10 object at 0x7f143362fd90>

Also, how would this affect create_unicode_buffer(), if the buffer is resized to a non-multiple of sizeof(c_wchar)?

Gedai: I’m not that familiar with the ctypes internals, but it looks like __len__() is implemented on the Array base class:

>>> type(b).__len__
<slot wrapper '__len__' of '_ctypes.Array' objects>

Maybe look for the implementation of this method: <https://docs.python.org/3/c-api/typeobj.html#c.PySequenceMethods.sq_length>.
msg259604 - (view) Author: Tamás Bence Gedai (beng94) * Date: 2016-02-04 23:49
I've added a patch, that solves the problem with the built-in len. Even if it turns out that this functionality is not needed, it was quite of a challenge to track down the issue, I've learned a lot. :)

Here are some functions, that I looked through, might be useful for someone, who'd like to look into this issue.

https://github.com/python/cpython/blob/master/Python/bltinmodule.c#L1443
static PyObject *
builtin_len(PyModuleDef *module, PyObject *obj)
/*[clinic end generated code: output=8e5837b6f81d915b input=bc55598da9e9c9b5]*/
{
    Py_ssize_t res;

    res = PyObject_Size(obj);
    if (res < 0 && PyErr_Occurred())
        return NULL;
    return PyLong_FromSsize_t(res);
}

https://github.com/python/cpython/blob/master/Objects/abstract.c#L42
Py_ssize_t
PyObject_Size(PyObject *o)
{
    /*...*/
    m = o->ob_type->tp_as_sequence;
    if (m && m->sq_length)
        return m->sq_length(o);
    /*...*/
}

https://github.com/python/cpython/blob/master/Modules/_ctypes/_ctypes.c#L4449
static PySequenceMethods Array_as_sequence = {
    Array_length,                               /* sq_length; */
    /*...*/
};

https://github.com/python/cpython/blob/master/Modules/_ctypes/_ctypes.c#L4442
static Py_ssize_t
Array_length(PyObject *myself)
{
    CDataObject *self = (CDataObject *)myself;
    return self->b_length;
}
msg259606 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-05 00:53
I can’t really comment on the patch, but I’m a bit worried that this is not the purpose of the b_length field.
msg259624 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2016-02-05 03:46
You can't reassign the array object's __class__, and you can't modify the array type itself, so I think modifying the internal b_length field of the object is a confused result. 

Even if you ignore this confusion, it's still not as simple as using the size in bytes as the length. That's what b_size is for, after all. The array length is the new size divided by the element size, which you can get from PyType_stgdict(dict->proto)->size. Also, you'd have to ensure it's only updating b_length for arrays, i.e. ArrayObject_Check(obj), since it makes no sense to modify the length of a simple type, struct, union, or [function] pointer.

However, I don't think this result is worth the confusion. ctypes buffers can grow, but arrays have a fixed size by design. There are already ways to access a resized buffer. For example, you can use the from_buffer method to create an instance of a new array type that has the desired length, or you can dereference a pointer for the new array type. So I'm inclined to close this issue as "not a bug".

Note: 
Be careful when resizing buffers that are shared across objects. Say you resize array "a" and share it as array "b" using from_buffer or a pointer dereference. Then later you resize "a" again. The underlying realloc might change the base address of the buffer, while "b" still uses the old address. For example:

    >>> a = (ctypes.c_int * 5)(*range(5))
    >>> ctypes.resize(a, 4 * 10)
    >>> b = ctypes.POINTER(ctypes.c_int * 10)(a)[0]
    >>> ctypes.addressof(a)
    20136704
    >>> ctypes.addressof(b)
    20136704
    >>> b._b_needsfree_ # b doesn't own the buffer
    0
    >>> b[:] # the reallocation is not zeroed
    [0, 1, 2, 3, 4, 0, 0, 32736, 48, 0]

Here's the problem to keep in mind:

    >>> ctypes.resize(a, 4 * 1000)
    >>> ctypes.addressof(a)
    22077952
    >>> ctypes.addressof(b)
    20136704
    >>> b[:] # garbage; maybe a segfault
    [1771815800, 32736, 1633841761, 540763495, 1652121965,
     543236197, 1718052211, 1953264993, 10, 0]
msg259638 - (view) Author: Tamás Bence Gedai (beng94) * Date: 2016-02-05 08:29
Thanks for the remarks, I think the issue can be closed as well.
msg259639 - (view) Author: Dustin Oprea (Dustin.Oprea) Date: 2016-02-05 08:40
I'm closing it. The ticket has been open two-years and no one else seemed to be interested in this issue until now, which leads me to believe that it's a PEBCAK/understanding issue. The rationale for why it's irrelevant seems sound. Thanks for digging through the code, Tamás. Thank you for your comments, Martin and Eryk.
History
Date User Action Args
2016-02-05 08:48:06eryksunsetresolution: not a bug
stage: patch review -> resolved
2016-02-05 08:40:40Dustin.Opreasetstatus: open -> closed

messages: + msg259639
2016-02-05 08:29:59beng94setmessages: + msg259638
2016-02-05 03:46:05eryksunsettype: enhancement

messages: + msg259624
nosy: + eryksun
2016-02-05 00:53:18martin.pantersetmessages: + msg259606
stage: patch review
2016-02-04 23:50:00beng94setfiles: + resize.patch
keywords: + patch
messages: + msg259604
2016-02-04 22:45:10martin.pantersetnosy: + martin.panter
messages: + msg259600
2016-02-04 16:38:45beng94setnosy: + beng94
messages: + msg259575
2014-04-22 16:12:26berker.peksagsetnosy: + meador.inge
2014-04-22 14:02:29Dustin.Opreacreate