Issue 22732: ctypes tests don't set correct restype for intptr_t functions (original) (raw)

Created on 2014-10-26 20:32 by steve.dower, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
test_ctypes.patch steve.dower,2014-10-26 20:32 review
Messages (6)
msg230035 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2014-10-26 20:32
The test_pass_pointer and test_int_pointer_arg tests are inconsistent between 32-bit and 64-bit builds because c_long is always 32 bits but the function being called may return a 64-bit value (it's a pointer, so c_long may truncate it). I'd prefer to have a c_intptr type to use, but this patch uses c_longlong in the test if that's the size of a pointer. Just looking for a quick review so I can check this into default. Not sure why it doesn't repro with VC10, but it certainly does with later compilers.
msg230049 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-27 00:11
Why not use c_size_t? Or is that incorrect in some cases?
msg230054 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2014-10-27 01:35
> Why not use c_size_t? Or is that incorrect in some cases? Strictly speaking, size_t doesn't have to encompass the entire addressable range, such as for architectures that use segmented addressing (e.g. near and far pointers). Practically speaking, no platform supported by ctypes makes this distinction, so c_size_t and c_ssize_t use the platform pointer size. https://hg.python.org/cpython/file/ab2c023a9432/Lib/ctypes/__init__.py#l459 OTOH, it's best to avoid accumulating layers of assumptions.
msg230095 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2014-10-27 19:20
I missed c_size_t somehow, but as eryksun says, it's not strictly the same thing. Of course, my current patch isn't the same thing either as it only supports 32-bit and 64-bit pointer sizes. I could make a bigger change to use c_void_p and compare its .value against None instead of 0 (which seems to be the only difference)?
msg230107 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-10-27 21:33
Well, if c_size_t is incorrect, then let's go with the patch.
msg230465 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-11-01 22:16
New changeset a944fe09fae8 by Steve Dower in branch 'default': #22732 ctypes tests don't set correct restype for intptr_t functions https://hg.python.org/cpython/rev/a944fe09fae8
History
Date User Action Args
2022-04-11 14:58:09 admin set github: 66921
2014-11-01 22:17:11 steve.dower set status: open -> closedresolution: fixedstage: patch review -> resolved
2014-11-01 22:16:10 python-dev set nosy: + python-devmessages: +
2014-10-27 21:33:37 pitrou set messages: +
2014-10-27 19:20:37 steve.dower set messages: +
2014-10-27 01:35:42 eryksun set nosy: + eryksunmessages: +
2014-10-27 00:11:07 pitrou set nosy: + pitroumessages: +
2014-10-26 20:32:06 steve.dower create