Skip to content

Conversation

magnified103
Copy link
Contributor

@magnified103 magnified103 commented Aug 31, 2025

This PR adds _validate_paramflags checks to the setattr of argtypes, which is likely the best way to fix this issue without performance regressions.

This PR also adds tests for invalid paramflags and argtypes.

@python-cla-bot
Copy link

python-cla-bot bot commented Aug 31, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Aug 31, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Signed-off-by: Nguyen Viet Dung <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Aug 31, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Copy link
Member

@encukou encukou 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! I have some suggestions.

Could you also test that with paramflag 2 (“output argument”), the argtype must be a pointer?

@magnified103
Copy link
Contributor Author

magnified103 commented Sep 8, 2025

I have made the requested changes; please review again.

return 0;
}
argtypes = info->argtypes;
if (argtypes == NULL || argtypes == Py_None) {
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to move the Py_None check to _ctypes_CFuncPtr_argtypes_set_impl? It would be nice to have this closer to the “user”, so the internals only need to consider NULL as special.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the Py_None check since it's already handled by _ctypes_CFuncPtr_argtypes_set_impl. The argtypes parameter will either be NULL or a non-None value.

@encukou encukou merged commit 1ce0553 into python:main Sep 10, 2025
81 of 83 checks passed
@encukou encukou added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Sep 10, 2025
@miss-islington-app
Copy link

Thanks @magnified103 for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @magnified103 for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @magnified103 and @encukou, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 1ce05537a3ebaf1e5c54505b2272d61bb6cf5de0 3.13

@encukou
Copy link
Member

encukou commented Sep 10, 2025

Thank you for the report and fix!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 10, 2025
…thonGH-138285)

(cherry picked from commit 1ce0553)

Co-authored-by: Dung Nguyen <[email protected]>
Signed-off-by: Nguyen Viet Dung <[email protected]>
Signed-off-by: Nguyen Viet Dung <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Yongzi Li <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Sep 10, 2025

GH-138742 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Sep 10, 2025
@bedevere-app
Copy link

bedevere-app bot commented Sep 10, 2025

GH-138745 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 10, 2025
@bedevere-app
Copy link

bedevere-app bot commented Sep 10, 2025

GH-138746 is a backport of this pull request to the 3.13 branch.

encukou pushed a commit to encukou/cpython that referenced this pull request Sep 10, 2025
…pes (pythonGH-138285)

(cherry picked from commit 1ce0553)

Co-authored-by: Dung Nguyen <[email protected]>
Signed-off-by: Nguyen Viet Dung <[email protected]>
Signed-off-by: Nguyen Viet Dung <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Yongzi Li <[email protected]>
Yhg1s pushed a commit that referenced this pull request Sep 18, 2025
…H-138285) (#138746)

(cherry picked from commit 1ce0553)

Signed-off-by: Nguyen Viet Dung <[email protected]>
Signed-off-by: Nguyen Viet Dung <[email protected]>
Co-authored-by: Dung Nguyen <[email protected]>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Yongzi Li <[email protected]>
encukou added a commit that referenced this pull request Oct 7, 2025
…H-138285) (GH-138742)

(cherry picked from commit 1ce0553)

Signed-off-by: Nguyen Viet Dung <[email protected]>
Co-authored-by: Yongzi Li <[email protected]>
Co-authored-by: Petr Viktorin <[email protected]>
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.

3 participants