Skip to content
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

Multiplication result converted to larger type #197

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Shivam7-1
Copy link
Contributor

To address the potential overflow issue in the multiplication of w * h * (bit16 ? 8 : 4) and casting it to size_t, I had perform the multiplication using size_t directly to avoid the overflow.
patch link: https://issuetracker.google.com/issues/322327444

Code modification ensures that the multiplication is done using the larger integer type (size_t) before casting the result to unsigned char*. Using size_t for the multiplication helps prevent overflow issues, as size_t is typically large enough to accommodate the size of memory in the system.

@Shivam7-1
Copy link
Contributor Author

Hi @lvandeve Could You Please Review Above PR

Thanks & Regards

@Shivam7-1
Copy link
Contributor Author

Hi @enh-google Could You Please Review Above PR

Thanks & Regards

@enh-google
Copy link
Contributor

(i only maintain Android's copy of this code, and we only use this file on the host, where we don't enable intsan anyway. i've forwarded this -- and the bug -- to the real zopfli maintainers internally though.)

@Shivam7-1
Copy link
Contributor Author

Hi @enh-google Okay Thanks 👍

@Shivam7-1
Copy link
Contributor Author

Hi @lvandeve Could You Please Review Above PR

Thanks & Regards

@lvandeve
Copy link
Collaborator

I pulled #198 which does the same, so this now has a conflict. Does #198 solve it?

@Shivam7-1
Copy link
Contributor Author

I pulled #198 which does the same, so this now has a conflict. Does #198 solve it?
Hi
Yes You are Right both are same
My mistake
Thanks

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