Skip to content

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Jan 31, 2024

@sobolevn sobolevn force-pushed the issue-114685-fill-info branch from 350a6e7 to 7e480af Compare January 31, 2024 12:47
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

See #60198. PyBuffer_FillInfo() with NULL as exporter is a fragile thing.

Copy link
Member

@serhiy-storchaka serhiy-storchaka 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.

Yet one nitpick. The code relies on the fact that b"abc" still exists after returning from buffer_fill_info(). It is so in CPython, but this is an implementation detail. It would be more reliable to explicitly keep a reference to it:

source = b'abc'
...
buffer_fill_info(source, ...)

@sobolevn
Copy link
Member Author

sobolevn commented Feb 4, 2024

Thank you! I always learn a lot from your reviews, very helpful.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants