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

Light refactor: create a common _Py_closerange API #84602

Closed
kevans91 mannequin opened this issue Apr 28, 2020 · 9 comments
Closed

Light refactor: create a common _Py_closerange API #84602

kevans91 mannequin opened this issue Apr 28, 2020 · 9 comments
Assignees
@gpshead
Labels
3.10 only security fixes topic-C-API type-feature A feature request or enhancement

Comments

@kevans91
Copy link
Mannequin

kevans91 mannequin commented Apr 28, 2020

BPO 40422
Nosy @gpshead, @vstinner, @miss-islington, @kevans91, @kevans91
PRs
  • bpo-40422: create a common _Py_closerange API #19754
  • bpo-40422: Move _Py_*_SUPPRESS_IPH bits into _Py_closerange #22672
  • bpo-40422: move _Py_closerange to core #22680
    		•

    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 = 'https://github.com/gpshead'
closed_at = <Date 2020-10-13.20:06:16.988>
created_at = <Date 2020-04-28.13:35:44.086>
labels = ['expert-C-API', 'type-feature', '3.10']
title = 'Light refactor: create a common _Py_closerange API'
updated_at = <Date 2020-10-13.20:06:16.986>
user = 'https://github.com/kevans91'

    bugs.python.org fields:

    activity = <Date 2020-10-13.20:06:16.986>
actor = 'vstinner'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2020-10-13.20:06:16.988>
closer = 'vstinner'
components = ['C API']
creation = <Date 2020-04-28.13:35:44.086>
creator = 'kevans91'
dependencies = []
files = []
hgrepos = []
issue_num = 40422
keywords = ['patch']
message_count = 9.0
messages = ['367530', '378447', '378464', '378466', '378468', '378497', '378536', '378573', '378574']
nosy_count = 5.0
nosy_names = ['gregory.p.smith', 'vstinner', 'miss-islington', 'kevans', 'kevans91']
pr_nums = ['19754', '22672', '22680']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue40422'
versions = ['Python 3.10']
    @kevans91
    Copy link
    Mannequin Author

    kevans91 mannequin commented Apr 28, 2020

    Such an API can be used for both os.closerange and subprocess, re-using much of os_closerange_impl. Pull request enroute.

    @kevans91 kevans91 mannequin added 3.9 only security fixes topic-C-API type-feature A feature request or enhancement labels Apr 28, 2020
    @miss-islington
    Copy link
    Contributor

    New changeset c230fde by Kyle Evans in branch 'master':
    bpo-40422: create a common _Py_closerange API (GH-19754)
    c230fde

    @gpshead gpshead closed this as completed Oct 11, 2020
    @gpshead gpshead closed this as completed Oct 11, 2020
    @vstinner
    Copy link
    Member

    Python/fileutils.c might be a better home for such helper function.

    You might move _Py_BEGIN_SUPPRESS_IPH / _Py_END_SUPPRESS_IPH macros into _Py_closerange(), even if currently it makes no diffence. (_Py_BEGIN_SUPPRESS_IPH is specific to Windows, os.closerange() uses it, and _posixsubprocess doesn't need it since it's not used on Windows.)

    See also the idea of reminding if the kernel supports the syscall or not:
    #22651 (review)

    @kevans91
    Copy link
    Mannequin Author

    kevans91 mannequin commented Oct 12, 2020

    Would you like that on a distinct issue, or is it ok to reuse this BPO since it's a location improvement of an API just introduced?

    I've got a local branch now that:

    1. moves the suppress IPH stuff into _Py_closerange
    2. moves the definition into Python/fileutils.c
    3. moves the declaration into Include/fileutils h

    @gpshead
    Copy link
    Member

    gpshead commented Oct 12, 2020

    Just reuse this bpo issue. I'll mark the PRs as "skip news"; don't worry about a new news blurb entry as it's all tied to the original one.

    @gpshead gpshead reopened this Oct 12, 2020
    @gpshead gpshead reopened this Oct 12, 2020
    @kevans91
    Copy link
    Mannequin Author

    kevans91 mannequin commented Oct 12, 2020

    Excellent, thank you.

    @gpshead
    Copy link
    Member

    gpshead commented Oct 12, 2020

    New changeset 64eb259 by Kyle Evans in branch 'master':
    bpo-40422: Move _Py_*_SUPPRESS_IPH bits into _Py_closerange (GH-22672)
    64eb259

    @vstinner
    Copy link
    Member

    New changeset 7992579 by Kyle Evans in branch 'master':
    bpo-40422: Move _Py_closerange to fileutils.c (GH-22680)
    7992579

    @vstinner
    Copy link
    Member

    Thanks Kyle Evans!

    @vstinner vstinner added 3.10 only security fixes and removed 3.9 only security fixes labels Oct 13, 2020
    @vstinner vstinner added 3.10 only security fixes and removed 3.9 only security fixes labels Oct 13, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @izbyshev izbyshev mentioned this issue May 4, 2022
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @gpshead gpshead

    Labels
    3.10 only security fixes topic-C-API type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

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