X Tutup
The Wayback Machine - https://web.archive.org/web/20260219140708/https://github.com/python/cpython/pull/118615
Skip to content

gh-118609: Add proper error check for framelocalsproxy#118615

Merged
gvanrossum merged 2 commits intopython:mainfrom
gaogaotiantian:fix-frameobject-error-checks
May 6, 2024
Merged

gh-118609: Add proper error check for framelocalsproxy#118615
gvanrossum merged 2 commits intopython:mainfrom
gaogaotiantian:fix-frameobject-error-checks

Conversation

@gaogaotiantian
Copy link
Member

@gaogaotiantian gaogaotiantian commented May 5, 2024

  • Completed error checks in frameobject.c
  • Fixed the compiler warning for unused variables
  • Polished the declarations a bit so the error checks are closer to the declared variable, and variables are declared in the necessary scope

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! One style nit, you can fix it or leave it.

(Maybe @sobolevn can review too, in case I missed anything -- I realize I am not up to speed with all the details of writing this kind of code any more.)

Comment on lines 383 to 385
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd swap these two lines, so that the Py_ReprEnter/Leave and the PyDict_New/Py_DECREF "brackets" properly nest. But this works too of course.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense. DECREF dct as soon as we are done with it and then leave repr. Fixed it.

@gvanrossum
Copy link
Member

(Maybe @sobolevn can review too, in case I missed anything -- I realize I am not up to speed with all the details of writing this kind of code any more.)

I meant @Eclips4 of course, since he reported the issue. (But Nikita is also welcome! :-)

@gvanrossum
Copy link
Member

I'll merge everything Monday morning (Pacific time) unless anyone says otherwise.

Copy link
Member

@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@gvanrossum gvanrossum merged commit 7528b84 into python:main May 6, 2024
@gaogaotiantian gaogaotiantian deleted the fix-frameobject-error-checks branch May 6, 2024 17:29
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments

X Tutup