Opened 5 years ago

Closed 5 years ago

#5373 defect closed fixed (fixed)

Make iocp 64-bit clean

Reported by: Antoine Pitrou Owned by: jesstess
Priority: normal Milestone:
Component: core Keywords:
Cc: Antoine Pitrou, jesstess Branch: branches/iocp-64bit-5373-2
branch-diff, diff-cov, branch-cov, buildbot
Author: antoine

Description

iocpsupport is in many places 64-bit unclean. This is due to mismatching declarations. First, HANDLE, SOCKET and ULONG_PTR are really pointer-sized, not int- or long- sized. Second, Cython always defines PY_SSIZE_T_CLEAN, which means functions such as PyObject_AsReadBuffer take a "Py_ssize_t *" parameter, not an "int *". Here is a patch. I've verified that it compiles fine under 64-bit Windows (it reduces the number of compiler warnings a lot), and twisted.internet.test passes fine with "-r iocp".

Attachments (2)

iocp64.patch (253.5 KB) - added by Antoine Pitrou 5 years ago.
iocp64-2.patch (254.4 KB) - added by Antoine Pitrou 5 years ago.

Download all attachments as: .zip

Change History (13)

Changed 5 years ago by Antoine Pitrou

Attachment: iocp64.patch added

comment:1 Changed 5 years ago by Antoine Pitrou

Cc: Antoine Pitrou added
Keywords: review added

(I've left the generated iocpsupport.c changes in the patch. Perhaps you want me to remove them for ease of review)

comment:2 Changed 5 years ago by jesstess

Owner: set to jesstess

comment:3 Changed 5 years ago by jesstess

Author: jesstess
Branch: branches/iocp-64bit-5373

(In [33236]) Branching to 'iocp-64bit-5373'

comment:4 Changed 5 years ago by jesstess

(In [33237]) Apply iocp64.patch by antoine.

refs #5373

comment:5 Changed 5 years ago by jesstess

Author: jesstessantoine
Cc: jesstess added
Keywords: review removed
Owner: changed from jesstess to Antoine Pitrou

Thanks for the detailed bug report and patch, antoine!

Here are the buildbot results for this branch.

Comparing the windows7-64-py2.7 compile warnings from a recent trunk build and this branch:

you'll see that a bunch of warnings have gone away, which is great, but 1 warning did get displaced instead of fixed:

twisted/internet/iocpreactor/iocpsupport/iocpsupport.c(3206) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data

is now

twisted/internet/iocpreactor/iocpsupport/iocpsupport.c(3438) : warning C4244: 'function' : conversion from 'Py_ssize_t' to 'DWORD', possible loss of data

Can you provide a patch against this branch for this warning in wsarecv.pxi:recv? After that I think this will be ready to merge.

Thanks again!

comment:6 Changed 5 years ago by Antoine Pitrou

Ok, here is an updated patch. I've only verified it compiles fine (it differs by a single cast from the previous patch).

Changed 5 years ago by Antoine Pitrou

Attachment: iocp64-2.patch added

comment:7 Changed 5 years ago by jesstess

Author: antoinejesstess, antoine
Branch: branches/iocp-64bit-5373branches/iocp-64bit-5373-2

(In [33238]) Branching to 'iocp-64bit-5373-2'

comment:8 Changed 5 years ago by jesstess

(In [33239]) Apply patch iocp64-2.patch by antoine.

refs #5373

comment:9 Changed 5 years ago by jesstess

Author: jesstess, antoineantoine
Keywords: review added
Owner: changed from Antoine Pitrou to jesstess

comment:10 Changed 5 years ago by jesstess

Keywords: review removed

Thanks for the new patch, antoine.

In the future, please submit subsequent patches again the branch instead of against trunk. Please also add the 'review' keyword when the ticket is ready to be reviewed again.

Here are the buildbot results for the new branch. Everything looks good, and the displaced compiler warning is gone:

I think this is ready to merge!

comment:11 Changed 5 years ago by jesstess

Resolution: fixed
Status: newclosed

(In [33241]) Merge iocp-64bit-5373-2

Author: antoine Reviewer: jesstess Fixes: #5373

Fix some iocpsupport declarations causing 64-bit compile warnings.

Note: See TracTickets for help on using tickets.