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

invalid exception handling in Lib/ctypes/macholib/dyld.py #63093

Closed
scoder opened this issue Aug 31, 2013 · 7 comments
Closed

invalid exception handling in Lib/ctypes/macholib/dyld.py #63093

scoder opened this issue Aug 31, 2013 · 7 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@scoder
Copy link
Contributor

scoder commented Aug 31, 2013

BPO 18893
Nosy @ronaldoussoren, @amauryfa, @abalkin, @scoder, @methane, @meadori, @berkerpeksag
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • issue18893.diff: 2 to 3 migration for framework_find error handling
  • 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 2016-09-26.20:06:40.176>
    created_at = <Date 2013-08-31.10:56:26.193>
    labels = ['3.7', 'ctypes', 'type-bug', 'library']
    title = 'invalid exception handling in Lib/ctypes/macholib/dyld.py'
    updated_at = <Date 2017-03-31.16:36:34.551>
    user = 'https://github.com/scoder'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:34.551>
    actor = 'dstufft'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-09-26.20:06:40.176>
    closer = 'berker.peksag'
    components = ['Library (Lib)', 'ctypes']
    creation = <Date 2013-08-31.10:56:26.193>
    creator = 'scoder'
    dependencies = []
    files = ['31560']
    hgrepos = []
    issue_num = 18893
    keywords = ['patch']
    message_count = 7.0
    messages = ['196629', '196630', '196772', '196807', '277324', '277455', '277456']
    nosy_count = 9.0
    nosy_names = ['ronaldoussoren', 'amaury.forgeotdarc', 'belopolsky', 'scoder', 'methane', 'meador.inge', 'python-dev', 'berker.peksag', 'madison.may']
    pr_nums = ['552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue18893'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7']

    @scoder
    Copy link
    Contributor Author

    scoder commented Aug 31, 2013

    The exception handling clauses in framework_find() are weird.

    def framework_find(fn, executable_path=None, env=None):
        """
        Find a framework using dyld semantics in a very loose manner.
    Will take input such as:
        Python
        Python.framework
        Python.framework/Versions/Current
    """
    try:
        return dyld_find(fn, executable_path=executable_path, env=env)
    except ValueError as e:
        pass
    fmwk_index = fn.rfind('.framework')
    if fmwk_index == -1:
        fmwk_index = len(fn)
        fn += '.framework'
    fn = os.path.join(fn, os.path.basename(fn[:fmwk_index]))
    try:
        return dyld_find(fn, executable_path=executable_path, env=env)
    except ValueError:
        raise e
    

    My guess is that this is left-over code from Py2.x. Since it doesn't make sense to catch an exception in the second clause just to re-raise it, I think the intention was really to re-raise the original exception caught in the first clause, which no longer works that way in Py3.

    The fix would then be to assign the exception to a new variable in the first except clause and re-raise that in the second.

    I found this problem because Cython rejected the module with a compile error about "e" being undefined in the last line.

    @scoder scoder added stdlib Python modules in the Lib dir topic-ctypes type-bug An unexpected behavior, bug, or error labels Aug 31, 2013
    @scoder
    Copy link
    Contributor Author

    scoder commented Aug 31, 2013

    changing title as it doesn't really look like a typo, more a "converto"

    @scoder scoder changed the title typo in Lib/ctypes/macholib/dyld.py invalid exception handling in Lib/ctypes/macholib/dyld.py Aug 31, 2013
    @ronaldoussoren
    Copy link
    Contributor

    Your analysis looks correct to me, that is "raise e" is supposed to raise the exception caught by the first try block.

    @MadisonMay
    Copy link
    Mannequin

    MadisonMay mannequin commented Sep 2, 2013

    Seems like a simple fix -- patch attached.

    @methane
    Copy link
    Member

    methane commented Sep 24, 2016

    lgtm

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 26, 2016

    New changeset e9f34d382eda by Berker Peksag in branch '3.5':
    Issue bpo-18893: Fix invalid exception handling in Lib/ctypes/macholib/dyld.py
    https://hg.python.org/cpython/rev/e9f34d382eda

    New changeset 708337cd8e6a by Berker Peksag in branch '3.6':
    Issue bpo-18893: Merge from 3.5
    https://hg.python.org/cpython/rev/708337cd8e6a

    New changeset b9d9c49d5b50 by Berker Peksag in branch 'default':
    Issue bpo-18893: Merge from 3.6
    https://hg.python.org/cpython/rev/b9d9c49d5b50

    @berkerpeksag
    Copy link
    Member

    Thanks!

    @berkerpeksag berkerpeksag added the 3.7 (EOL) end of life label Sep 26, 2016
    @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
    3.7 (EOL) end of life stdlib Python modules in the Lib dir topic-ctypes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants