Skip to content

Handle C long size on Windows. #497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 26, 2017
Merged

Handle C long size on Windows. #497

merged 1 commit into from
Jun 26, 2017

Conversation

filmor
Copy link
Member

@filmor filmor commented Jun 16, 2017

On Windows 64bit systems, a C long is a 32 bit integer while the rest
of the world has agreed on making sizeof(long) == sizeof(void*).

What does this implement/fix? Explain your changes.

New attempt on #376

Any other comments?

@vmuriart Do you have a test-case for PR #376?

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

@filmor
Handle C long size on Windows.
cd16bab 
On Windows 64bit systems, a C `long` is a 32 bit integer while the rest
of the world has agreed on making `sizeof(long) == sizeof(void*)`.
@mention-bot
Copy link

@filmor, thanks! @vmuriart, @tiran, @brianlloyd, @hsoft and @tonyroberts, please review this.

@den-run-ai
Copy link
Contributor

@filmor tp_flags is c_long in Python 2.7 and unsigned c_long in Python 3.6. This may not fit into CLR int (System.Int32) on *NIX and should be unsigned UInt32 or UInt64 for PYTHON3.

@den-run-ai
Copy link
Contributor

https://docs.python.org/3/c-api/typeobj.html
https://docs.python.org/2/c-api/typeobj.html

@codecov
Copy link

codecov bot commented Jun 19, 2017 •
edited
Loading

Codecov Report

Merging #497 into master will decrease coverage by 0.05%.
The diff coverage is 65%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #497      +/-   ##
==========================================
- Coverage   76.38%   76.33%   -0.06%     
==========================================
  Files          64       65       +1     
  Lines        5573     5582       +9     
  Branches      896      898       +2     
==========================================
+ Hits         4257     4261       +4     
- Misses       1021     1024       +3     
- Partials      295      297       +2
Flag Coverage Δ
#setup_linux 75.71% <ø> (ø) ⬆️
#setup_windows 75.69% <65%> (-0.06%) ⬇️
Impacted Files Coverage Δ
src/runtime/metatype.cs 70.73% <100%> (ø) ⬆️
src/runtime/clrobject.cs 96.15% <100%> (ø) ⬆️
src/runtime/runtime.cs 90.87% <100%> (+0.03%) ⬆️
src/runtime/Util.cs 37.5% <37.5%> (ø)
src/runtime/managedtype.cs 50% <50%> (ø) ⬆️
src/runtime/typemanager.cs 84.03% <75%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12af794...cd16bab. Read the comment docs.

@filmor
Copy link
Member Author

filmor commented Jun 19, 2017

Does it matter, though? Bitwise it doesn't make a difference, and I'm reading the actual value of tp_flags using the ReadInt functions, so they will just interpret too large numbers as negative.

@filmor filmor merged commit 0ce6301 into pythonnet:master Jun 26, 2017
testrunner123 pushed a commit to testrunner123/pythonnet that referenced this pull request Sep 22, 2017
@filmor @testrunner123
Handle C long size on Windows. (pythonnet#497)
3c28ca3 
On Windows 64bit systems, a C `long` is a 32 bit integer while the rest
of the world has agreed on making `sizeof(long) == sizeof(void*)`.
@filmor filmor deleted the handle-clong-windows branch September 26, 2019 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@den-run-ai den-run-ai den-run-ai approved these changes

@tonyroberts tonyroberts Awaiting requested review from tonyroberts

@vmuriart vmuriart Awaiting requested review from vmuriart

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@filmor @mention-bot @den-run-ai