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

PyObject_GetAttrString and tp_getattr do not agree #83801

Open
petdance mannequin opened this issue Feb 13, 2020 · 4 comments
Open

PyObject_GetAttrString and tp_getattr do not agree #83801

petdance mannequin opened this issue Feb 13, 2020 · 4 comments
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@petdance
Copy link
Mannequin

petdance mannequin commented Feb 13, 2020

BPO 39620
Nosy @loewis, @encukou, @serhiy-storchaka, @ammaraskar, @petdance

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-02-13.03:23:38.855>
labels = ['expert-C-API', 'type-feature']
title = 'PyObject_GetAttrString and tp_getattr do not agree'
updated_at = <Date 2021-06-22.14:42:14.566>
user = 'https://github.com/petdance'

bugs.python.org fields:

activity = <Date 2021-06-22.14:42:14.566>
actor = 'petr.viktorin'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['C API']
creation = <Date 2020-02-13.03:23:38.855>
creator = 'petdance'
dependencies = []
files = []
hgrepos = []
issue_num = 39620
keywords = []
message_count = 4.0
messages = ['361929', '361930', '361931', '361936']
nosy_count = 5.0
nosy_names = ['loewis', 'petr.viktorin', 'serhiy.storchaka', 'ammar2', 'petdance']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue39620'
versions = []

@petdance
Copy link
Mannequin Author

petdance mannequin commented Feb 13, 2020

PyObject_GetAttrString(PyObject *v, const char *name)

typedef PyObject *(*getattrfunc)(PyObject *, char *)

The outer PyObject_GetAttrString takes a const char *name, but then casts away the const when calling the underlying tp_getattr. This means that an underlying function would be free to modify or free() the char* passed in to it, which might be, for example, a string literal, which would be a Bad Thing.

The setattr function pair has the same problem.

The API doc at https://docs.python.org/3/c-api/typeobj.html says that the tp_getattr and tp_setattr slots are deprecated. If they're not going away soon, I would think this should be addressed.

Fixing this in the cPython code by making tp_getattr and tp_setattr take const char * pointers would be simple. I don't have any idea how much outside code it would affect.

@petdance petdance mannequin added topic-C-API type-feature A feature request or enhancement labels Feb 13, 2020
@ammaraskar
Copy link
Member

Note that there was an earlier attempt to make it const
af68c87

but this was reverted as part of 15e6274

@petdance
Copy link
Mannequin Author

petdance mannequin commented Feb 13, 2020

Do you know why it was reverted? (Granted, it was 15 years ago...)

It looks like the original changeset is trying to address at least two different problems with non-const string literals. My ticket here is focusing only on getattrfunc and setattrfunc. The handling of all the kwlist is a separate issue, that I'm about to write up as a different ticket.

@serhiy-storchaka
Copy link
Member

It was reverted because it is backward incompatible change. It breaks a code which assigns to the tp_getattr field without explicit type cast to getattrfunc.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants