Skip to content

(p)ereallocarray introduction. #8882

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
wants to merge 1 commit into from

Conversation

devnexen
Copy link
Member

Sort of #8871 follow-up but on the zend part.

void *p;

size = zend_safe_address_guarded(nmemb, size, 0);
p = _erealloc(ptr, size ZEND_FILE_LINE_RELAY_CC ZEND_FILE_LINE_ORIG_RELAY_CC);
Copy link
Member

Choose a reason for hiding this comment

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

This is essentially exactly the same as safe_erealloc(ptr, nmemb, size, 0), no?

Copy link
Member Author

Choose a reason for hiding this comment

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

good point :)

Copy link
Member

Choose a reason for hiding this comment

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

But wasn't the point of reallocarray that in case of an overflow for m * n we don't touch the memory? I'm not completely sure what erealloc does with size 0.

Copy link
Member

Choose a reason for hiding this comment

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

I just noticed, zend_safe_address_guarded triggers a zend_error_noreturn on overflow, so checking for 0 doesn't make sense. I wonder if this PR makes sense at all, if we can essentially just do safe_erealloc(ptr, nmemb, size, 0).

Copy link
Member Author

Choose a reason for hiding this comment

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

yes you re right

Copy link
Member

Choose a reason for hiding this comment

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

You could still switch the few cases form erealloc to erealloc_safe 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

sure I prefer a new fresh PR ;-)

@devnexen devnexen force-pushed the zend_ereallocarray_intro branch from c0683b1 to 4e615e3 Compare July 2, 2022 15:46
Sort of php#8871 follow-up but on the zend part.
@devnexen devnexen force-pushed the zend_ereallocarray_intro branch from 4e615e3 to c426783 Compare July 2, 2022 17:42
@devnexen devnexen closed this Jul 4, 2022
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.

2 participants