Skip to content

Conversation

iluuu1994
Copy link
Member

Closes GH-8808

@cmb69
Copy link
Member

cmb69 commented Jun 17, 2022

I'm not too happy with the error message generally, and duplicating "bytes" doesn't make it better, in my opinion. Can't we come up with something else? OTOH, certainly not a hill to die upon for me.

@iluuu1994
Copy link
Member Author

I'll try to come up with something better.

@mvorisek
Copy link
Contributor

also the number should be converted to human format (with KB, MB, GB, ... unit)

#else
zend_mm_safe_error(heap, "Out of memory (allocated %zu) (tried to allocate %zu bytes)", heap->real_size, ZEND_MM_PAGE_SIZE * pages_count);
zend_mm_safe_error(heap, "Out of memory (allocated %zu bytes) (tried to allocate %zu bytes)", heap->real_size, ZEND_MM_PAGE_SIZE * pages_count);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
zend_mm_safe_error(heap, "Out of memory (allocated %zu bytes) (tried to allocate %zu bytes)", heap->real_size, ZEND_MM_PAGE_SIZE * pages_count);
zend_mm_safe_error(heap, "Out of memory while trying to allocate %zu bytes (allocated %zu bytes)", ZEND_MM_PAGE_SIZE * pages_count, heap->real_size);

Maybe?

@Girgias Girgias merged commit cd363a9 into php:master Jun 21, 2022
@Girgias
Copy link
Member

Girgias commented Jun 21, 2022

I've merged because we can always improve the error message later

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.

better error reporting
5 participants