Skip to content
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.

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

Fix Windows compile issues #67

Closed
wants to merge 3 commits into from
Closed

Fix Windows compile issues #67

wants to merge 3 commits into from

Conversation

scott-8
Copy link

@scott-8 scott-8 commented Apr 2, 2020

Fixes applied based on discussions here:
#61
mkleehammer/pyodbc#663

Please test to make sure the changes to python.cc work on Linux.

Fixes applied based on discussions here:
src-d#61
mkleehammer/pyodbc#663

Signed-off-by: Scott <[email protected]>
Copy link
Collaborator

@vmarkovtsev vmarkovtsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for digging this!

Comment on lines +152 to +153
long l=PyLong_AsLong(cache_obj);
cache = reinterpret_cast<C *>(reinterpret_cast<intptr_t>( &l ));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no good: instead of converting an integer to a cache pointer you pretend that a local variable's address is a cache pointer. The size of long on Windows equals to 4 bytes while the pointer size is 8 bytes. Can you please replace PyLong_AsLong with PyLong_AsLongLong here?

Suggested change
long l=PyLong_AsLong(cache_obj);
cache = reinterpret_cast<C *>(reinterpret_cast<intptr_t>( &l ));
cache = reinterpret_cast<C *>(reinterpret_cast<intptr_t>(
PyLong_AsLongLong(cache_obj)));

Copy link
Author

@scott-8 scott-8 Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unable to get this to work. I continue to get python.cc(152): error C2440: 'reinterpret_cast': cannot convert from '__int64' to 'intptr_t'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed reinterpret_cast<intptr_t>(PyLong_AsLongLong(cache_obj)) to reinterpret_cast<intptr_t *>(PyLong_AsLongLong(cache_obj)) and it compiled successfully, I'm not exactly sure about the implications of that, though.

emd_relaxed.h Outdated Show resolved Hide resolved
emd.h Outdated Show resolved Hide resolved
scott-8 and others added 2 commits April 2, 2020 21:42
Co-Authored-By: Vadim Markovtsev <[email protected]>
Co-Authored-By: Vadim Markovtsev <[email protected]>
@scott-8 scott-8 closed this Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants