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.

Sign up for GitHub

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

Jump to bottom

[C API] marshal.h must not use FILE* type in the limited C API #89637

Closed
vstinner opened this issue Oct 14, 2021 · 6 comments
Closed

[C API] marshal.h must not use FILE* type in the limited C API #89637

vstinner opened this issue Oct 14, 2021 · 6 comments
Labels
3.11 only security fixes topic-C-API

Comments

@vstinner
Copy link
Member

BPO 45474
Nosy @vstinner, @encukou, @miss-islington
PRs
  • bpo-45474: Fix the limited C API of marshal.h #28956
  • bpo-45474: Exclude all of marshal.h if Py_LIMITED_API is defined #29061
    		•

    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 = <Date 2021-10-20.09:33:07.251>
created_at = <Date 2021-10-14.20:09:23.656>
labels = ['expert-C-API', '3.11']
title = '[C API] marshal.h must not use FILE* type in the limited C API'
updated_at = <Date 2021-10-20.09:33:07.250>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2021-10-20.09:33:07.250>
actor = 'petr.viktorin'
assignee = 'none'
closed = True
closed_date = <Date 2021-10-20.09:33:07.251>
closer = 'petr.viktorin'
components = ['C API']
creation = <Date 2021-10-14.20:09:23.656>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 45474
keywords = ['patch']
message_count = 6.0
messages = ['403941', '403958', '403985', '403993', '404308', '404408']
nosy_count = 3.0
nosy_names = ['vstinner', 'petr.viktorin', 'miss-islington']
pr_nums = ['28956', '29061']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue45474'
versions = ['Python 3.11']
    @vstinner
    Copy link
    Member Author

    Include/marshal.h defines 2 functions with FILE* argument in the limited C API, whereas the PEP-384 disallows that:

    "In addition, functions expecting FILE* are not part of the ABI, to avoid depending on a specific version of the Microsoft C runtime DLL on Windows."
    https://www.python.org/dev/peps/pep-0384/

    PyAPI_FUNC(void) PyMarshal_WriteLongToFile(long, FILE *, int);
PyAPI_FUNC(void) PyMarshal_WriteObjectToFile(PyObject *, FILE *, int);

    I propose to exclude these functions from the limited C API.

    Hopefully, they are not part of the documented stable ABI.

    @vstinner vstinner added 3.11 only security fixes topic-C-API labels Oct 14, 2021
    @vstinner
    Copy link
    Member Author

    New changeset af1083e by Victor Stinner in branch 'main':
    bpo-45474: Fix the limited C API of marshal.h (GH-28956)
    af1083e

    @encukou
    Copy link
    Member

    encukou commented Oct 15, 2021

    Just note that these were *not* part of the limited API, which is defined in Misc/stable_abi.txt rather than the #ifdefs: https://docs.python.org/3.10/c-api/stable.html#stable
    If they were, the functions would need to remain in the stable ABI.

    @vstinner
    Copy link
    Member Author

    Petr:

    Just note that these were *not* part of the limited API, which is defined in Misc/stable_abi.txt rather than the #ifdefs: https://docs.python.org/3.10/c-api/stable.html#stable
    If they were, the functions would need to remain in the stable ABI.

    Thanks for double checking. I wasn't sure ;-)
    #28956 (comment)

    @encukou
    Copy link
    Member

    encukou commented Oct 19, 2021

    Nothing from marshal.h is part of the limited API, and nothing from there is exported in the stable ABI DLL.
    The entire file should be in #ifndef Py_LIMITED_API, rather than just a part.

    @encukou encukou reopened this Oct 19, 2021
    @encukou encukou reopened this Oct 19, 2021
    @miss-islington
    Copy link
    Contributor

    New changeset 98fa3b5 by Petr Viktorin in branch 'main':
    bpo-45474: Exclude all of marshal.h if Py_LIMITED_API is defined (GH-29061)
    98fa3b5

    @encukou encukou closed this as completed Oct 20, 2021
    @encukou encukou closed this as completed Oct 20, 2021
    @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
    Assignees
    No one assigned
    Labels
    3.11 only security fixes topic-C-API
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @encukou @miss-islington