Skip to content

Commit beb4d36

Browse files
karldinghardbyte
karlding
authored and
hardbyte
committed
Fix stuct packing in build_bcm_header on 32-bit (#573)
* Fix stuct packing in build_bcm_header on 32-bit Previously build_bcm_header relied upon adding a "0q" at the end of the struct format string in order to force a 8-byte wide alignment on the struct, in order to interop with the structure defined in the Linux headers: struct bcm_msg_head { __u32 opcode; __u32 flags; __u32 count; struct bcm_timeval ival1, ival2; canid_t can_id; __u32 nframes; struct can_frame frames[0]; }; The Python code assumed that alignof(long long) == 8 bytes in order to accomplish this. However, on certain 32-bit platforms, this assumption is not true, and alignof(long long) == 4. As the struct module does not provide logic to derive alignments of its types, this changes the packing logic to use ctypes. This exposes the alignment of the struct members, allowing us to determine whether padding bytes are necessary. Fixes #470 * Call BCM factory upon module initialization In order to prevent the BCM factory method from being called each time a BCM header struct is created, we create the class when the module gets created. * Add SocketCAN interface tests for BCM factory Add a SocketCAN interface test file with tests for validating the BcmMsgHead class created by bcm_header_factory on various platforms. * Clean up comments referencing packing via struct Clean up the old comment in build_bcm_header referencing the old packing code that called into Python's struct library. This now uses ctypes, so and the class is created via the bcm_header_factory factory function, so we move the comment there instead.
1 parent 8504a4e commit beb4d36

File tree

2 files changed

+335
-21
lines changed

2 files changed

+335
-21
lines changed

‎can/interfaces/socketcan/socketcan.py‎

Lines changed: 101 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,84 @@ def get_addr(sock, channel):
6767
return struct.pack("HiLL", AF_CAN, idx, 0, 0)
6868

6969

70+
# Setup BCM struct
71+
def bcm_header_factory(fields, alignment=8):
72+
curr_stride = 0
73+
results = []
74+
pad_index = 0
75+
for field in fields:
76+
field_alignment = ctypes.alignment(field[1])
77+
field_size = ctypes.sizeof(field[1])
78+
79+
# If the current stride index isn't a multiple of the alignment
80+
# requirements of this field, then we must add padding bytes until we
81+
# are aligned
82+
while curr_stride % field_alignment != 0:
83+
results.append(("pad_{}".format(pad_index), ctypes.c_uint8))
84+
pad_index += 1
85+
curr_stride += 1
86+
87+
# Now can it fit?
88+
# Example: If this is 8 bytes and the type requires 4 bytes alignment
89+
# then we can only fit when we're starting at 0. Otherwise, we will
90+
# split across 2 strides.
91+
#
92+
# | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 |
93+
results.append(field)
94+
curr_stride += field_size
95+
96+
# Add trailing padding to align to a multiple of the largest scalar member
97+
# in the structure
98+
while curr_stride % alignment != 0:
99+
results.append(("pad_{}".format(pad_index), ctypes.c_uint8))
100+
pad_index += 1
101+
curr_stride += 1
102+
103+
return type("BcmMsgHead", (ctypes.Structure,), {"_fields_": results})
104+
105+
# The fields definition is taken from the C struct definitions in
106+
# <linux/can/bcm.h>
107+
#
108+
# struct bcm_timeval {
109+
# long tv_sec;
110+
# long tv_usec;
111+
# };
112+
#
113+
# /**
114+
# * struct bcm_msg_head - head of messages to/from the broadcast manager
115+
# * @opcode: opcode, see enum below.
116+
# * @flags: special flags, see below.
117+
# * @count: number of frames to send before changing interval.
118+
# * @ival1: interval for the first @count frames.
119+
# * @ival2: interval for the following frames.
120+
# * @can_id: CAN ID of frames to be sent or received.
121+
# * @nframes: number of frames appended to the message head.
122+
# * @frames: array of CAN frames.
123+
# */
124+
# struct bcm_msg_head {
125+
# __u32 opcode;
126+
# __u32 flags;
127+
# __u32 count;
128+
# struct bcm_timeval ival1, ival2;
129+
# canid_t can_id;
130+
# __u32 nframes;
131+
# struct can_frame frames[0];
132+
# };
133+
BcmMsgHead = bcm_header_factory(
134+
fields=[
135+
("opcode", ctypes.c_uint32),
136+
("flags", ctypes.c_uint32),
137+
("count", ctypes.c_uint32),
138+
("ival1_tv_sec", ctypes.c_long),
139+
("ival1_tv_usec", ctypes.c_long),
140+
("ival2_tv_sec", ctypes.c_long),
141+
("ival2_tv_usec", ctypes.c_long),
142+
("can_id", ctypes.c_uint32),
143+
("nframes", ctypes.c_uint32),
144+
]
145+
)
146+
147+
70148
# struct module defines a binary packing format:
71149
# https://docs.python.org/3/library/struct.html#struct-format-strings
72150
# The 32bit can id is directly followed by the 8bit data link count
@@ -118,27 +196,29 @@ def build_can_frame(msg):
118196
return CAN_FRAME_HEADER_STRUCT.pack(can_id, msg.dlc, flags) + data
119197

120198

121-
def build_bcm_header(opcode, flags, count, ival1_seconds, ival1_usec, ival2_seconds, ival2_usec, can_id, nframes):
122-
# == Must use native not standard types for packing ==
123-
# struct bcm_msg_head {
124-
# __u32 opcode; -> I
125-
# __u32 flags; -> I
126-
# __u32 count; -> I
127-
# struct timeval ival1, ival2; -> llll ...
128-
# canid_t can_id; -> I
129-
# __u32 nframes; -> I
130-
bcm_cmd_msg_fmt = "@3I4l2I0q"
131-
132-
return struct.pack(bcm_cmd_msg_fmt,
133-
opcode,
134-
flags,
135-
count,
136-
ival1_seconds,
137-
ival1_usec,
138-
ival2_seconds,
139-
ival2_usec,
140-
can_id,
141-
nframes)
199+
def build_bcm_header(
200+
opcode,
201+
flags,
202+
count,
203+
ival1_seconds,
204+
ival1_usec,
205+
ival2_seconds,
206+
ival2_usec,
207+
can_id,
208+
nframes,
209+
):
210+
result = BcmMsgHead(
211+
opcode=opcode,
212+
flags=flags,
213+
count=count,
214+
ival1_tv_sec=ival1_seconds,
215+
ival1_tv_usec=ival1_usec,
216+
ival2_tv_sec=ival2_seconds,
217+
ival2_tv_usec=ival2_usec,
218+
can_id=can_id,
219+
nframes=nframes,
220+
)
221+
return ctypes.string_at(ctypes.addressof(result), ctypes.sizeof(result))
142222

143223

144224
def build_bcm_tx_delete_header(can_id, flags):

‎test/test_socketcan.py‎

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
"""
2+
Test functions in `can.interfaces.socketcan.socketcan`.
3+
"""
4+
import unittest
5+
6+
try:
7+
from unittest.mock import Mock
8+
from unittest.mock import patch
9+
from unittest.mock import call
10+
except ImportError:
11+
from mock import Mock
12+
from mock import patch
13+
from mock import call
14+
15+
import ctypes
16+
17+
from can.interfaces.socketcan.socketcan import bcm_header_factory
18+
19+
20+
class SocketCANTest(unittest.TestCase):
21+
def setUp(self):
22+
self._ctypes_sizeof = ctypes.sizeof
23+
self._ctypes_alignment = ctypes.alignment
24+
25+
@patch("ctypes.sizeof")
26+
@patch("ctypes.alignment")
27+
def test_bcm_header_factory_32_bit_sizeof_long_4_alignof_long_4(
28+
self, ctypes_sizeof, ctypes_alignment
29+
):
30+
"""This tests a 32-bit platform (ex. Debian Stretch on i386), where:
31+
32+
* sizeof(long) == 4
33+
* sizeof(long long) == 8
34+
* alignof(long) == 4
35+
* alignof(long long) == 4
36+
"""
37+
38+
def side_effect_ctypes_sizeof(value):
39+
type_to_size = {
40+
ctypes.c_longlong: 8,
41+
ctypes.c_long: 4,
42+
ctypes.c_uint8: 1,
43+
ctypes.c_uint16: 2,
44+
ctypes.c_uint32: 4,
45+
ctypes.c_uint64: 8,
46+
}
47+
return type_to_size[value]
48+
49+
def side_effect_ctypes_alignment(value):
50+
type_to_alignment = {
51+
ctypes.c_longlong: 4,
52+
ctypes.c_long: 4,
53+
ctypes.c_uint8: 1,
54+
ctypes.c_uint16: 2,
55+
ctypes.c_uint32: 4,
56+
ctypes.c_uint64: 4,
57+
}
58+
return type_to_alignment[value]
59+
60+
ctypes_sizeof.side_effect = side_effect_ctypes_sizeof
61+
ctypes_alignment.side_effect = side_effect_ctypes_alignment
62+
63+
fields = [
64+
("opcode", ctypes.c_uint32),
65+
("flags", ctypes.c_uint32),
66+
("count", ctypes.c_uint32),
67+
("ival1_tv_sec", ctypes.c_long),
68+
("ival1_tv_usec", ctypes.c_long),
69+
("ival2_tv_sec", ctypes.c_long),
70+
("ival2_tv_usec", ctypes.c_long),
71+
("can_id", ctypes.c_uint32),
72+
("nframes", ctypes.c_uint32),
73+
]
74+
BcmMsgHead = bcm_header_factory(fields)
75+
76+
expected_fields = [
77+
("opcode", ctypes.c_uint32),
78+
("flags", ctypes.c_uint32),
79+
("count", ctypes.c_uint32),
80+
("ival1_tv_sec", ctypes.c_long),
81+
("ival1_tv_usec", ctypes.c_long),
82+
("ival2_tv_sec", ctypes.c_long),
83+
("ival2_tv_usec", ctypes.c_long),
84+
("can_id", ctypes.c_uint32),
85+
("nframes", ctypes.c_uint32),
86+
# We expect 4 bytes of padding
87+
("pad_0", ctypes.c_uint8),
88+
("pad_1", ctypes.c_uint8),
89+
("pad_2", ctypes.c_uint8),
90+
("pad_3", ctypes.c_uint8),
91+
]
92+
self.assertEqual(expected_fields, BcmMsgHead._fields_)
93+
94+
@patch("ctypes.sizeof")
95+
@patch("ctypes.alignment")
96+
def test_bcm_header_factory_32_bit_sizeof_long_4_alignof_long_8(
97+
self, ctypes_sizeof, ctypes_alignment
98+
):
99+
"""This tests a 32-bit platform (ex. Raspbian Stretch on armv7l), where:
100+
101+
* sizeof(long) == 4
102+
* sizeof(long long) == 8
103+
* alignof(long) == 4
104+
* alignof(long long) == 8
105+
"""
106+
107+
def side_effect_ctypes_sizeof(value):
108+
type_to_size = {
109+
ctypes.c_longlong: 8,
110+
ctypes.c_long: 4,
111+
ctypes.c_uint8: 1,
112+
ctypes.c_uint16: 2,
113+
ctypes.c_uint32: 4,
114+
ctypes.c_uint64: 8,
115+
}
116+
return type_to_size[value]
117+
118+
def side_effect_ctypes_alignment(value):
119+
type_to_alignment = {
120+
ctypes.c_longlong: 8,
121+
ctypes.c_long: 4,
122+
ctypes.c_uint8: 1,
123+
ctypes.c_uint16: 2,
124+
ctypes.c_uint32: 4,
125+
ctypes.c_uint64: 8,
126+
}
127+
return type_to_alignment[value]
128+
129+
ctypes_sizeof.side_effect = side_effect_ctypes_sizeof
130+
ctypes_alignment.side_effect = side_effect_ctypes_alignment
131+
132+
fields = [
133+
("opcode", ctypes.c_uint32),
134+
("flags", ctypes.c_uint32),
135+
("count", ctypes.c_uint32),
136+
("ival1_tv_sec", ctypes.c_long),
137+
("ival1_tv_usec", ctypes.c_long),
138+
("ival2_tv_sec", ctypes.c_long),
139+
("ival2_tv_usec", ctypes.c_long),
140+
("can_id", ctypes.c_uint32),
141+
("nframes", ctypes.c_uint32),
142+
]
143+
BcmMsgHead = bcm_header_factory(fields)
144+
145+
expected_fields = [
146+
("opcode", ctypes.c_uint32),
147+
("flags", ctypes.c_uint32),
148+
("count", ctypes.c_uint32),
149+
("ival1_tv_sec", ctypes.c_long),
150+
("ival1_tv_usec", ctypes.c_long),
151+
("ival2_tv_sec", ctypes.c_long),
152+
("ival2_tv_usec", ctypes.c_long),
153+
("can_id", ctypes.c_uint32),
154+
("nframes", ctypes.c_uint32),
155+
# We expect 4 bytes of padding
156+
("pad_0", ctypes.c_uint8),
157+
("pad_1", ctypes.c_uint8),
158+
("pad_2", ctypes.c_uint8),
159+
("pad_3", ctypes.c_uint8),
160+
]
161+
self.assertEqual(expected_fields, BcmMsgHead._fields_)
162+
163+
@patch("ctypes.sizeof")
164+
@patch("ctypes.alignment")
165+
def test_bcm_header_factory_64_bit_sizeof_long_4_alignof_long_4(
166+
self, ctypes_sizeof, ctypes_alignment
167+
):
168+
"""This tests a 64-bit platform (ex. Ubuntu 18.04 on x86_64), where:
169+
170+
* sizeof(long) == 8
171+
* sizeof(long long) == 8
172+
* alignof(long) == 8
173+
* alignof(long long) == 8
174+
"""
175+
176+
def side_effect_ctypes_sizeof(value):
177+
type_to_size = {
178+
ctypes.c_longlong: 8,
179+
ctypes.c_long: 8,
180+
ctypes.c_uint8: 1,
181+
ctypes.c_uint16: 2,
182+
ctypes.c_uint32: 4,
183+
ctypes.c_uint64: 8,
184+
}
185+
return type_to_size[value]
186+
187+
def side_effect_ctypes_alignment(value):
188+
type_to_alignment = {
189+
ctypes.c_longlong: 8,
190+
ctypes.c_long: 8,
191+
ctypes.c_uint8: 1,
192+
ctypes.c_uint16: 2,
193+
ctypes.c_uint32: 4,
194+
ctypes.c_uint64: 8,
195+
}
196+
return type_to_alignment[value]
197+
198+
ctypes_sizeof.side_effect = side_effect_ctypes_sizeof
199+
ctypes_alignment.side_effect = side_effect_ctypes_alignment
200+
201+
fields = [
202+
("opcode", ctypes.c_uint32),
203+
("flags", ctypes.c_uint32),
204+
("count", ctypes.c_uint32),
205+
("ival1_tv_sec", ctypes.c_long),
206+
("ival1_tv_usec", ctypes.c_long),
207+
("ival2_tv_sec", ctypes.c_long),
208+
("ival2_tv_usec", ctypes.c_long),
209+
("can_id", ctypes.c_uint32),
210+
("nframes", ctypes.c_uint32),
211+
]
212+
BcmMsgHead = bcm_header_factory(fields)
213+
214+
expected_fields = [
215+
("opcode", ctypes.c_uint32),
216+
("flags", ctypes.c_uint32),
217+
("count", ctypes.c_uint32),
218+
# We expect 4 bytes of padding
219+
("pad_0", ctypes.c_uint8),
220+
("pad_1", ctypes.c_uint8),
221+
("pad_2", ctypes.c_uint8),
222+
("pad_3", ctypes.c_uint8),
223+
("ival1_tv_sec", ctypes.c_long),
224+
("ival1_tv_usec", ctypes.c_long),
225+
("ival2_tv_sec", ctypes.c_long),
226+
("ival2_tv_usec", ctypes.c_long),
227+
("can_id", ctypes.c_uint32),
228+
("nframes", ctypes.c_uint32),
229+
]
230+
self.assertEqual(expected_fields, BcmMsgHead._fields_)
231+
232+
233+
if __name__ == "__main__":
234+
unittest.main()

0 commit comments

Comments
 (0)