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

resource max value represented as signed when should be unsigned #54126

Open
bitdancer opened this issue Sep 22, 2010 · 6 comments
Open

resource max value represented as signed when should be unsigned #54126

bitdancer opened this issue Sep 22, 2010 · 6 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@bitdancer
Copy link
Member

BPO 9917
Nosy @loewis, @giampaolo, @devdanzin, @bitdancer, @wiggin15, @vadmium
Files
  • issue9917.diff
    		•

    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 2010-09-22.01:42:54.603>
labels = ['extension-modules', 'type-bug', '3.9', '3.10', '3.11']
title = 'resource max value represented as signed when should be unsigned'
updated_at = <Date 2021-12-17.00:35:29.310>
user = 'https://github.com/bitdancer'

    bugs.python.org fields:

    activity = <Date 2021-12-17.00:35:29.310>
actor = 'ajaksu2'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules']
creation = <Date 2010-09-22.01:42:54.603>
creator = 'r.david.murray'
dependencies = []
files = ['40613']
hgrepos = []
issue_num = 9917
keywords = ['patch']
message_count = 6.0
messages = ['117123', '117138', '228039', '251771', '251802', '255942']
nosy_count = 9.0
nosy_names = ['loewis', 'nnorwitz', 'mdr0', 'sable', 'giampaolo.rodola', 'ajaksu2', 'r.david.murray', 'wiggin15', 'martin.panter']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue9917'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']
    @bitdancer
    Copy link
    Member Author

    Breaking out the library bug discussed in bpo-678264 from the test bug the issue is about.

    See msg14344 and msg116479.

    @bitdancer bitdancer added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 22, 2010
    @bitdancer
    Copy link
    Member Author

    And here is Martin's summary of the issue:

    I think we really should create new issues for any remaining problems.

    AFAICT, the remaining problems are:

    • resource.RLIM_INFINITY is -1 on Linux, when it is meant to be
      a really large value
    • (as you reported) getrlimit returns -1 to indicate RLIM_INFINITY.

    I think the core of the problem is that the resource module considers
    rlim_t to be a signed type (or at least representable in "long long").
    Using FromUnsignedLongLong in the appropriate place might solve the
    issue.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Sep 30, 2014

    @david one for your TODO list or does this rightfully belong with somebody else?

    @wiggin15
    Copy link
    Mannequin

    wiggin15 mannequin commented Sep 28, 2015

    getrlimit still returns -1 as 'max' when limit is unlimited and for RLIM_INFINITY because rlim_t is considered signed.

    The zshell project decides whether rlim_t is signed/unsigned (and also whether it is long or long long) in the configure step: https://github.com/zsh-users/zsh/blob/8b84419f45298ee564bd6fa2b531c8991b2a1983/configure.ac#L1859

    On Linux, rlim_t is unisnged long long so the conversion should be done using PyLong_FromUnsignedLongLong (for RLIM_INFINITY) and using 'KK' instead of 'LL' in 'rlimit2py'.

    IMHO the best way to fix this is to add configure steps like in zsh and then adding ifdefs to resource.c - in rlimit2py and near PyModule_AddObject of RLIM_INFINITY

    @wiggin15
    Copy link
    Mannequin

    wiggin15 mannequin commented Sep 28, 2015

    I'm submitting a patch for review. I tested this and verified the values on Redhat and Unbuntu (both x86 and x64), Mac OS X (x64) and AIX (32-bit process on ppc64).
    'configure' and 'pyconfig.h.in' were auto-generated with autoconf and autoheader, respectively.
    A test for this patch (along with a fix for a different test failure) is in msg117130

    @vadmium
    Copy link
    Member

    vadmium commented Dec 5, 2015

    Are there platforms where rlim_t is signed? Posix defines it as unsigned: <http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_resource.h.html\>.

    Also there are subtle differences between the PyLong_AsUnsigned[. . .] and the current signed versions. The unsigned version does not call __int__() so you would no longer be able to pass a float().

    Also this could cause compatibility problems with code that was written for earlier documentation, which said to use -1 to mean unlimited. Perhaps that is why the code has rlim_* & RLIM_INFINITY masking.

    @vadmium vadmium added extension-modules C modules in the Modules dir and removed stdlib Python modules in the Lib dir labels Dec 5, 2015
    @devdanzin devdanzin mannequin added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes labels Dec 17, 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.9 only security fixes 3.10 only security fixes 3.11 only security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @bitdancer @vadmium