Skip to content

Port "improve _gdImageFillTiled() internal function" #17356

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

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Jan 4, 2025

This improvement was done for libgd 2.1.0[1], and the erroneous calculation has been fixed as of libgd 2.2.0[2].

While we're at it we also add the overflow checks of external libgd; these are not really necessary, since .sx * .sy overflow was already prevented when the image has been created, and since we're using safe_emalloc() the struct seg overflow is also prevented. It should be noted that overflow2() prevents int overflow, while safe_emalloc() prevents size_t overflow, so the former is more restrictive. For parity with external libgd, this still appears to be a good thing.

[1] libgd/libgd@86a5deb
[2] libgd/libgd@e87ec88

This improvement was done for libgd 2.1.0[1], and the erroneous
calculation has been fixed as of libgd 2.2.0[2].

While we're at it we also add the overflow checks of external libgd;
these are not really necessary, since `.sx * .sy` overflow was already
prevented when the image has been created, and since we're using
`safe_emalloc()` the `struct seg` overflow is also prevented.  It
should be noted that `overflow2()` prevents `int` overflow, while
`safe_emalloc()` prevents `size_t` overflow, so the former is more
restrictive.  For parity with external libgd, this still appears to be
a good thing.

[1] <libgd/libgd@86a5deb>
[2] <libgd/libgd@e87ec88>
@cmb69 cmb69 requested a review from devnexen as a code owner January 4, 2025 13:57
@devnexen
Copy link
Member

devnexen commented Jan 4, 2025

Looks like a bit of a fix more than anything ?

@cmb69
Copy link
Member Author

cmb69 commented Jan 4, 2025

Looks like a bit of a fix more than anything ?

Like I wrote, the overflow checks are not necessary. And allocating a single large array instead of allocating an array per row is only an improvement.

That said, I'm fine with targeting PHP-8.3.

@devnexen
Copy link
Member

devnexen commented Jan 4, 2025

my bad did not read your comment. Either way I m fine, once the very minor issue is fixed.

@cmb69
Copy link
Member Author

cmb69 commented Jan 4, 2025

once the very minor issue is fixed.

Argh, I hate when that happens (unused variable).

@cmb69 cmb69 merged commit 2c49f52 into php:master Jan 7, 2025
10 checks passed
@cmb69
Copy link
Member Author

cmb69 commented Jan 7, 2025

I've merged into master; I think that's good enough, and if someone complains, we can still consider to backport to lower branches.

@cmb69 cmb69 deleted the cmb/_gdImageFillTiled branch January 7, 2025 00:24
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