Skip to content

Conversation

@SahillMulani
Copy link

@SahillMulaniSahillMulani 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

@SahillMulaniSahillMulani changed the title Assertion Error FixGH-82565: Fixed a possible assertion errorApr 3, 2023
@arhadthedevarhadthedev 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

@SahillMulaniSahillMulaniApr 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 .

@AlexWaygoodAlexWaygood removed the tests Tests in the Lib/test dir label Apr 3, 2023
@Eclips4
Copy link
Member

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 freeto 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

@SahillMulani@bedevere-bot@Eclips4@arhadthedev@AlexWaygood