-
Notifications
You must be signed in to change notification settings - Fork 8.2k
display: ensure samples check display api func return value + ltdc read/write rectangle check #99166
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
base: main
Are you sure you want to change the base?
display: ensure samples check display api func return value + ltdc read/write rectangle check #99166
Conversation
Ensure that display api function return values are checked. Signed-off-by: Alain Volmat <[email protected]>
Ensure that x,y/width-height rectangle mentioned in display_read and display_write are actually part of the framebuffer, otherwise return an error. Signed-off-by: Alain Volmat <[email protected]>
|
| ret = display_blanking_off(display_dev); | ||
| if (ret < 0 && ret != -ENOSYS) { | ||
| LOG_ERR("Failed to turn blanking off (error %d)", ret); | ||
| return 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return ret
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I have been wondering about what to return and couldn't really find a common behavior between samples. For example few lines above, if the device is not ready, gives an error message but also return 0.
In some other sample, a non 0 value is returned but most common behavior seems to return 0.
In this PR I tried as much as possible to stick to what is done within each sample even if I feel it weird to return 0 on error. I would be ok to return ret in case of error in fact but, this has to be done commonly I think with other samples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Samples should return 0.
| ret = display_blanking_off(display_dev); | ||
| if (ret < 0 && ret != -ENOSYS) { | ||
| LOG_ERR("Failed to turn blanking off (error %d)", ret); | ||
| return ret; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return ret; | |
| return 0; |



Following PR #98719, add proper display API function return value checks in all samples in order to fail if an error happen.
Moreover, within the LTDC driver, ensure that the rectangle given (x,y / width-height) properly fit into the framebuffer in order xto avoid OOB access.
Such check is currently done within the LTDC only, however I think that some of those checks should be done within the API functions themself to ensure a first level of proper parameter validity. In order to have proper behavior of all display drivers, some level of framework level checks can be done, removing amount of duplicated code between drivers.
I will prepare a PR for that purpose and propose it.