Skip to content

GH-82565: Fixed a possible assertion error #103206

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

Closed

Conversation

SahillMulani
Copy link

@SahillMulani SahillMulani commented Apr 3, 2023

GH-82565 Fixed a possible assertion error

[bpo-38384-pickle-assert] #16606

Please check the below code test cases

File without read and readline

  • Please help me with this test case

File with bad read and without readline

    class F:  
        read = bad_property
    self.assertRaises(ZeroDivisionError, self.Unpickler, F())

File with bad readline and without read

    class F: 
        readline = bad_property
    self.assertRaises(ZeroDivisionError, self.Unpickler, F())

Fixed a possible assertion error
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@ghost
Copy link

ghost commented Apr 3, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@SahillMulani SahillMulani changed the title Assertion Error Fix GH-82565: Fixed a possible assertion error Apr 3, 2023
@arhadthedev arhadthedev added the tests Tests in the Lib/test dir label Apr 3, 2023
(void)_PyObject_LookupAttr(file, &_Py_ID(read), &self->read);
(void)_PyObject_LookupAttr(file, &_Py_ID(readline), &self->readline);
if (!self->readline || !self->read) {
if (_PyObject_LookupAttrId(file, &PyId_read, &self->read) <= 0 ||
Copy link
Member

Choose a reason for hiding this comment

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

Seems that you forgot to define a PyId_read and PyId_readline.
However, have you try to a built python with this patch manually?

Copy link
Author

Choose a reason for hiding this comment

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

I have fixed this issue in the latest commit ✅

Please have a look , Thanks

Copy link
Member

Choose a reason for hiding this comment

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

Seems that tests keep falling.
You sure that you made necessary changes?

Copy link
Author

@SahillMulani SahillMulani Apr 3, 2023

Choose a reason for hiding this comment

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

Below changes are made by me :

image

All other changes are imported as it is from #16606

Therefore, I am not sure where exactly the code is failing .

@AlexWaygood AlexWaygood removed the tests Tests in the Lib/test dir label Apr 3, 2023
@SahillMulani SahillMulani requested a review from Eclips4 April 4, 2023 11:35
@Eclips4
Copy link
Member

Eclips4 commented Apr 4, 2023

Hi!
Can you check the CI/CD actions?
It's says something like

D:\a\cpython\cpython\Modules\_pickle.c(1648,52): error C2065: 'PyId_readline': undeclared identifier [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

You forgot to define some identifiers, so, CI/CD fails.

@SahillMulani
Copy link
Author

@Eclips4 As mentioned ,
This wasn't my code and I had imported from #16606 ,
Therefore , closing the Pull Request .
Thanks for your time

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.

5 participants