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 pymod on 32 bit architectures #1892

Merged
merged 3 commits into from Dec 27, 2018

Conversation

Projects
None yet
4 participants
@SteVwonder
Copy link
Member

SteVwonder commented Dec 23, 2018

PyLong_FromVoidPtr overflows on 32-bit architectures.
Switch to using PyLong_FromLong.
Include support for architectures where a pointer is larger than long (i.e., PyLong_FromLongLong).

Also includes two minor cleanups.

Fixes #1801

@SteVwonder SteVwonder requested review from garlick and trws Dec 23, 2018

@SteVwonder SteVwonder force-pushed the SteVwonder:fix-32bit-python branch from 14b6bb2 to d6f8a9b Dec 23, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 23, 2018

Codecov Report

Merging #1892 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1892      +/-   ##
==========================================
- Coverage   80.13%   80.11%   -0.03%     
==========================================
  Files         196      196              
  Lines       35060    35064       +4     
==========================================
- Hits        28094    28090       -4     
- Misses       6966     6974       +8
Impacted Files Coverage Δ
src/modules/pymod/py_mod.c 66.66% <100%> (+1.6%) ⬆️
src/modules/barrier/barrier.c 76.55% <0%> (-2.07%) ⬇️
src/common/libflux/mrpc.c 87.55% <0%> (-1.21%) ⬇️
src/broker/module.c 82.47% <0%> (-0.27%) ⬇️
src/modules/connector-local/local.c 73.62% <0%> (-0.15%) ⬇️
src/cmd/flux-event.c 77.97% <0%> (ø) ⬆️
@garlick
Copy link
Member

garlick left a comment

Works on the armv7l (raspberry pi)!

Thanks @SteVwonder

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Dec 26, 2018

This seems to do no harm, and given that we don't have many python modules in the wild, not very risky. What say we rebase and merge this? (I'll do the rebase in a couple hours if there are no objections).

SteVwonder added some commits Dec 23, 2018

pymod: fix for 32-bit architectures
PyLong_FromVoidPtr overflows on 32-bit architectures.  Switch to using
PyLong_FromLong.  Include support for architectures where a pointer is
larger than `long` (i.e., PyLong_FromLongLong).

Fixes #1801

@garlick garlick force-pushed the SteVwonder:fix-32bit-python branch from d6f8a9b to 6eb3df4 Dec 27, 2018

@chu11

This comment has been minimized.

Copy link
Contributor

chu11 commented Dec 27, 2018

LGTM, this ready to go?

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Dec 27, 2018

@chu11 chu11 merged commit 161f86e into flux-framework:master Dec 27, 2018

3 checks passed

codecov/patch 100% of diff hit (target 80.13%)
Details
codecov/project Absolute coverage decreased by -0.02% but relative coverage increased by +19.86% compared to 913c104
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@SteVwonder SteVwonder deleted the SteVwonder:fix-32bit-python branch Feb 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.