Skip to content

bpo-27195: Handle NULL format in memoryview objects.#9615

Closed
taralx wants to merge 1 commit into
python:masterfrom
taralx:patch-1
Closed

bpo-27195: Handle NULL format in memoryview objects.#9615
taralx wants to merge 1 commit into
python:masterfrom
taralx:patch-1

Conversation

@taralx

@taralx taralx commented Sep 28, 2018

Copy link
Copy Markdown

See https://bugs.python.org/issue27195 for details of the crash. Backport PRs will come after this one is approved.

https://bugs.python.org/issue27195

@the-knights-who-say-ni

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@taralx

taralx commented Sep 28, 2018

Copy link
Copy Markdown
Author

CLA signed.

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tolist() still crashes in Python 2.

self.assertEqual(c.format, "H")
self.assertEqual(d.format, "H")

def test_minimal_buffer(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This test is repeated in 6 test classes:

test_minimal_buffer (test.test_memoryview.ArrayMemorySliceSliceTest) ... ok
test_minimal_buffer (test.test_memoryview.ArrayMemorySliceTest) ... ok
test_minimal_buffer (test.test_memoryview.ArrayMemoryviewTest) ... ok
test_minimal_buffer (test.test_memoryview.BytesMemorySliceSliceTest) ... ok
test_minimal_buffer (test.test_memoryview.BytesMemorySliceTest) ... ok
test_minimal_buffer (test.test_memoryview.BytesMemoryviewTest) ... ok

Move it to more appropriate place, so that it will not be repeated. For example in OtherTest.

Also, this test is not crashed on Python 3.

Add a reference to the issue.

corresponding field in the buffer structure was not set. Per documentation
of the new-style buffer protocol, an unset ``format`` field is to be
interpreted as ``"B"``, so that is now returned in this case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove the empty line. Unfortunately multiparagraph news entries are not supported.

@bedevere-bot

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

@taralx

taralx commented Oct 12, 2018

Copy link
Copy Markdown
Author

What is tolist()? This is not mentioned in the bug.

@serhiy-storchaka

Copy link
Copy Markdown
Member

A method of memoryview. It was mentioned in a comment to alternate PR #8415.

@taralx

taralx commented Oct 15, 2018

Copy link
Copy Markdown
Author

So the test doesn't crash on Python 3 because of this: https://github.com/python/cpython/blob/master/Objects/memoryobject.c#L526

Shall I close this and reopen as just the 2.7 fix?

@vstinner

Copy link
Copy Markdown
Member

I removed the " needs backport to 3.6" label, the 3.6 branch no longer accept bugfixes (only security fixes are accepted): https://devguide.python.org/#status-of-python-branches

@taralx

taralx commented Jan 10, 2019

Copy link
Copy Markdown
Author

Going to close this for now. I've migrated off 2.7 at this point, so I'll let someone else take that up if they're concerned.

@taralx taralx closed this Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants