classification
Title: ctypes tests don't set correct restype for intptr_t functions
Type: Stage: resolved
Components: ctypes, Tests Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steve.dower Nosy List: amaury.forgeotdarc, belopolsky, eryksun, meador.inge, pitrou, python-dev, steve.dower
Priority: normal Keywords: patch

Created on 2014-10-26 20:32 by steve.dower, last changed 2014-11-01 22:17 by steve.dower. 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
2014-11-01 22:17:11steve.dowersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2014-11-01 22:16:10python-devsetnosy: + python-dev
messages: + msg230465
2014-10-27 21:33:37pitrousetmessages: + msg230107
2014-10-27 19:20:37steve.dowersetmessages: + msg230095
2014-10-27 01:35:42eryksunsetnosy: + eryksun
messages: + msg230054
2014-10-27 00:11:07pitrousetnosy: + pitrou
messages: + msg230049
2014-10-26 20:32:06steve.dowercreate