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

[XDP] Handle incorrect range of tiles usage for memory tiles settings #8657

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

Conversation

vipangul
Copy link
Collaborator

Problem solved by the commit

[CR-1222763] Incorrect usage of range of memory tiles was resulting into segmentation faults.

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

CR-1222763 - UG1076 has documentation error and needs an update for tile based settings of memory tiles.

How problem was solved, alternative solutions (if any) and why they were rejected

Parsing of user settings are in try-catch block. However, this was skipped as invalid string to int conversion was not thrown and re-caught.

Risks (if any) associated the changes in the commit

Low.

What has been tested and how, request additional testing if necessary

Verified on VEK280.
Verified that user now doesn't see crash and appropriate warning is emitted.
Verified that correct usage of memory tile settings are parsed correctly by plugin.

Documentation impact (if any)

Yes. UG1076 needs an update.

Signed-off-by: Vinod Pangul <[email protected]>
@vipangul vipangul requested a review from jvillarre as a code owner December 13, 2024 21:14
@vipangul vipangul requested a review from pgschuey December 13, 2024 21:14
Comment on lines +388 to +392
try {
return static_cast<uint8_t>(std::stoi(input));
} catch (const std::exception&) {
throw;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change is a no-op. It has no effect.

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.

2 participants