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

ci(cunit): set generic argument and return types #125

Merged
merged 3 commits into from
Oct 14, 2022

Conversation

P403n1x87
Copy link
Owner

@P403n1x87 P403n1x87 commented Jul 14, 2022

Description of the Change

Depending on how Python is compiled on certain architectures, not giving types to C functions using ctypes can lead to mangled values (e.g. a 64-bit pointer having its top 32 bits set to 1). This is a highly likely source of segmentation faults, and they were discovered using the Python builds from the setup-python action.

With this change, we ensure that all arguments and return values get a default type. We use c_void_p for any pointer and c_long for any non-pointer argument/return value, to ensure that the passed bits are not altered by unwanted conversions.

Alternate Designs

N. A.

Regressions

N. A.

Verification Process

Existing test suite.

@codecov
Copy link

codecov bot commented Jul 14, 2022

Codecov Report

Base: 69.16% // Head: 69.75% // Increases project coverage by +0.58% 🎉

Coverage data is based on head (0ff07a7) compared to base (3dc9961).
Patch has no changes to coverable lines.

❗ Current head 0ff07a7 differs from pull request most recent head 72d12d8. Consider uploading reports for the commit 72d12d8 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            devel     #125      +/-   ##
==========================================
+ Coverage   69.16%   69.75%   +0.58%     
==========================================
  Files          23       23              
  Lines        2384     2394      +10     
  Branches      696      697       +1     
==========================================
+ Hits         1649     1670      +21     
+ Misses        416      405      -11     
  Partials      319      319              
Impacted Files Coverage Δ
src/logging.c 73.46% <0.00%> (-8.54%) ⬇️
src/py_proc_list.c 75.57% <0.00%> (-2.30%) ⬇️
src/stack.h 83.57% <0.00%> (+0.71%) ⬆️
src/py_thread.c 73.33% <0.00%> (+1.93%) ⬆️
src/py_proc.c 63.35% <0.00%> (+2.43%) ⬆️
src/py_string.h 69.23% <0.00%> (+2.56%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@P403n1x87 P403n1x87 force-pushed the ci/setup-python-workaround branch 2 times, most recently from 49f8acd to 50d7a25 Compare July 18, 2022 08:47
@P403n1x87 P403n1x87 force-pushed the devel branch 2 times, most recently from 46d96d6 to 09deb5f Compare July 23, 2022 10:23
@P403n1x87 P403n1x87 force-pushed the ci/setup-python-workaround branch from 46cff25 to 11b2805 Compare July 25, 2022 11:29
@P403n1x87
Copy link
Owner Author

Closing because it doesn't seem to actually work around actions/setup-python#442.

@P403n1x87 P403n1x87 closed this Sep 14, 2022
@P403n1x87 P403n1x87 reopened this Oct 14, 2022
@P403n1x87 P403n1x87 force-pushed the ci/setup-python-workaround branch 4 times, most recently from 23d1778 to 0ff07a7 Compare October 14, 2022 15:27
@P403n1x87 P403n1x87 force-pushed the ci/setup-python-workaround branch from 0ff07a7 to 72d12d8 Compare October 14, 2022 18:23
@P403n1x87 P403n1x87 changed the title ci: add setup-python workaround on Linux ci(cuni): set generic argument and return types Oct 14, 2022
@P403n1x87 P403n1x87 marked this pull request as ready for review October 14, 2022 18:32
@P403n1x87
Copy link
Owner Author

P403n1x87 commented Oct 14, 2022

@dsame I'm tagging you in case you'd be interested in the current solution, which is based on your valuable analysis 🙇 .

@P403n1x87 P403n1x87 changed the title ci(cuni): set generic argument and return types ci(cunit): set generic argument and return types Oct 14, 2022
@P403n1x87 P403n1x87 merged commit a633faf into devel Oct 14, 2022
@P403n1x87 P403n1x87 deleted the ci/setup-python-workaround branch October 14, 2022 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant