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

LibGfx: Return BPP for all supported image formats #2237

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

Conversation

shlyakpavel
Copy link
Contributor

This commit moves some of the actual code from the header file to the CPP file to reduce header size and improve encapsulation. Additionally, it ensures that the bpp is correctly returned for all supported image formats, which may help prevent potential runtime crashes in the future.

Comment on lines -124 to -125
default:
VERIFY_NOT_REACHED();
Copy link
Contributor

Choose a reason for hiding this comment

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

this was neat ersatz for pattern matching, probably worth keeping in the new location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current code ensures that all values are validated at compile-time, which provides a significant advantage: it prevents the possibility of introducing a new color space without explicitly adding support for it in this function. This approach eliminates the risk of runtime crashes due to unhandled cases, as any missing implementation will result in a compile-time error rather than a runtime failure.

Copy link
Member

Choose a reason for hiding this comment

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

Does any caller call this method with untrusted user input? Could any caller be reasonably created to call with untrusted user input? And by untrusted user input I am including image data from an undecided image, as well as information that comes from IPC.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case, since it returns, the VERIFY_NOT_REACHED() still should be added - just after the switch.

This change aims to prevent potential runtime crashes in the future.
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.

4 participants