classification
Title: Nested ctypes 'BigEndianStructure' fails
Type: enhancement Stage: resolved
Components: ctypes Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: theller Nosy List: Alexander.Belopolsky, amaury.forgeotdarc, castironpi, haypo, python-dev, terry.reedy, theller, vladris
Priority: normal Keywords: patch

Created on 2008-11-21 10:09 by castironpi, last changed 2011-07-13 20:20 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
ng36.py castironpi, 2008-11-21 10:09 Repro example and traceback
issue4376_test.diff vladris, 2011-07-12 01:35 Unit test to reproduce the issue review
issue4376_fix.diff vladris, 2011-07-13 16:40 Patch (+minor refactoring) review
Messages (17)
msg76171 - (view) Author: Aaron Brady (castironpi) Date: 2008-11-21 10:09
Nested 'BigEndianStructure' fails in 2.5 and 2.6.:

TypeError: This type does not support other endian

Example and traceback in attached file.
msg76513 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2008-11-27 19:20
Currently, the _fields_ of ctypes Structures with non-native byte order
can only contain simple types (like int, char, but not pointers), and
arrays of those.

Since this is all Python code (in Lib/ctypes/endian.py) it should be
possible to extend the code to handle other types as well.

If we do this, it must be decided if a Structure (call it 'part' for
this discussion) of some byte order is contained in a _field_ of a
non-native Structure type:
- Should 'part' be inserted as is, leading to a total structure of
fields with mixed byte order?

- Should a new type be created from the 'part' _fields_, with also
non-native byte-order?

Other approaches would be possible as well...

Here is a simple patch that implements the first approach; I have not
tested if it works correctly:

Index: _endian.py
===================================================================
--- _endian.py	(revision 67045)
+++ _endian.py	(working copy)
@@ -17,6 +17,8 @@
     except AttributeError:
         if type(typ) == _array_type:
             return _other_endian(typ._type_) * typ._length_
+        if issubclass(typ, Structure):
+            return typ
         raise TypeError("This type does not support other endian: %s" %
typ)
 
 class _swapped_meta(type(Structure)):
msg76514 - (view) Author: Thomas Heller (theller) * (Python committer) Date: 2008-11-27 19:21
Correction:
>> Since this is all Python code (in Lib/ctypes/_endian.py) it should be
msg100831 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-11 03:19
ping! This issue is still open.
msg100874 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-03-11 16:24
theller> - Should 'part' be inserted as is, [possibly] leading 
theller>   to a total structure of fields with mixed byte order?

+1 for this option.
msg112757 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-08-04 04:49
The test could be build from the attached example.
msg140167 - (view) Author: Vlad Riscutia (vladris) Date: 2011-07-12 01:35
Added unit test to reproduce the issue (covers both big endian and little endian structures so _other_endian function is hit on any machine).

Test currently fails without fix and passes with proposed fix in place.
msg140172 - (view) Author: Vlad Riscutia (vladris) Date: 2011-07-12 02:50
Also added diff for fix:

- Implemented proposed issubclass(typ, Structure) solution
- Refactored _other_endian function because we used to getattr, catch exception, then check if type is array/structure. I believe exception throwing should not be on normal code path so I replaced try-except with a check for hasattr
- Removed a unittest which becomes deprecated with this fix (unittest asserts getting _other_endian for nested struct raises exception which is no longer the case)
msg140177 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-07-12 08:25
Is there a unit test about the actual feature: that the bytes are actually swapped in the structure?

For example, with a
class T(BigEndianStructure): _fields_ = [("a", c_int), ("b", c_int)]
cast a pointer to T into a pointer to c_int, and read the values.
msg140273 - (view) Author: Vlad Riscutia (vladris) Date: 2011-07-13 15:32
New patch containing unittest that actually tests the feature.

I would appreciate it if someone can try this on a bigendian machine.
msg140277 - (view) Author: Vlad Riscutia (vladris) Date: 2011-07-13 15:44
Changed c_int in tests to c_int32 just to be on the safe side.
msg140279 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-07-13 16:05
I don't like your test because it depends on system endian:

+            if sys.byteorder == "little":
+                struct.menu.spam = 0x000000FF
+            else:
+                struct.menu.spam = 0xFF000000

I would prefer a test creating a structure from a byte string. Something like:
---------------------------
import ctypes

class Nested(ctypes.BigEndianStructure):
    _fields_ = (
        ('x', ctypes.c_uint32),
        ('y', ctypes.c_uint32),
    )

class TestStruct(ctypes.BigEndianStructure):
    _fields_ = (
        ('point', Nested),
    )

data = b'\0\0\0\1\0\0\0\2'
assert len(data) == ctypes.sizeof(TestStruct)
obj = ctypes.cast(data, ctypes.POINTER(TestStruct))[0]
assert obj.point.x == 1
assert obj.point.y == 2
---------------------------

Use b'\1\0\0\0\2\0\0\0' for little endian.
msg140282 - (view) Author: Vlad Riscutia (vladris) Date: 2011-07-13 16:22
But if you set raw memory to let's say b'\0\0\0\1', when you look at the c_int value afterwards, won't it be different on little endian and big endian machines?
msg140284 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-07-13 16:26
> But if you set raw memory to let's say b'\0\0\0\1',
> when you look at the c_int value afterwards, won't it
> be different on little endian and big endian machines?

A big endian structure is supposed to read and write memory in the... big endian order, not in the host endian.
msg140285 - (view) Author: Vlad Riscutia (vladris) Date: 2011-07-13 16:40
You're right. I was busy swapping bytes in my head and missed that :)
msg140299 - (view) Author: Roundup Robot (python-dev) Date: 2011-07-13 19:48
New changeset 72e73fa03124 by Victor Stinner in branch '3.2':
Close #4376: ctypes now supports nested structures in a endian different than
http://hg.python.org/cpython/rev/72e73fa03124

New changeset 637210b9d054 by Victor Stinner in branch 'default':
(merge 3.2) Close #4376: ctypes now supports nested structures in a endian
http://hg.python.org/cpython/rev/637210b9d054

New changeset a9d0fab19d5e by Victor Stinner in branch '2.7':
Close #4376: ctypes now supports nested structures in a endian different than
http://hg.python.org/cpython/rev/a9d0fab19d5e
msg140301 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-07-13 20:20
I commited your fix, thanks Vlad!
History
Date User Action Args
2011-07-13 20:20:52hayposetmessages: + msg140301
2011-07-13 19:48:27python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg140299

resolution: fixed
stage: test needed -> resolved
2011-07-13 16:40:09vladrissetfiles: + issue4376_fix.diff

messages: + msg140285
2011-07-13 16:39:07vladrissetfiles: - issue4376_fix.diff
2011-07-13 16:26:27hayposetmessages: + msg140284
2011-07-13 16:24:31Jake.Coffmansetnosy: - Jake.Coffman
2011-07-13 16:22:16vladrissetmessages: + msg140282
2011-07-13 16:05:38hayposetmessages: + msg140279
2011-07-13 15:44:46vladrissetfiles: + issue4376_fix.diff

messages: + msg140277
2011-07-13 15:44:10vladrissetfiles: - issue4376_fix.diff
2011-07-13 15:32:14vladrissetfiles: + issue4376_fix.diff

messages: + msg140273
2011-07-13 15:30:16vladrissetfiles: - issue4376_fix.diff
2011-07-12 08:25:46amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg140177
2011-07-12 02:50:20vladrissetfiles: + issue4376_fix.diff

messages: + msg140172
2011-07-12 01:35:09vladrissetfiles: + issue4376_test.diff

nosy: + vladris
messages: + msg140167

keywords: + patch
2011-02-23 22:19:56Jake.Coffmansetnosy: + Jake.Coffman
2010-08-04 04:49:34terry.reedysetversions: + Python 3.2, - Python 2.6, Python 2.5
nosy: + terry.reedy

messages: + msg112757

type: compile error -> enhancement
stage: test needed
2010-03-11 16:24:58Alexander.Belopolskysetnosy: + Alexander.Belopolsky
messages: + msg100874
2010-03-11 03:19:35hayposetnosy: + haypo
messages: + msg100831
2008-11-27 19:21:58thellersetmessages: + msg76514
2008-11-27 19:20:45thellersetmessages: + msg76513
2008-11-21 10:09:30castironpicreate