Skip to content

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