Skip to content

Make auto-discovery error message handling more failsafe #110

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

Merged
merged 2 commits into from
Apr 25, 2022
Merged

Make auto-discovery error message handling more failsafe #110

merged 2 commits into from
Apr 25, 2022

Conversation

pcolberg
Copy link
Contributor

@pcolberg pcolberg commented Apr 22, 2022

Currently it is too easy to fill the stringstream err_ss with an error
message but forget to assign the contents to the string err_str.

Reduce scope of stringstream err_ss to where it is used to make such
mistakes more obvious. In C++20, this may be replaced with std::format().

This is a prerequisite of #108 and #103.

The stringstream err_ss is only used within the function, and the
contents must be assigned to err_str which is returned to the caller.

This amends #100

Signed-off-by: Peter Colberg <[email protected]>
@pcolberg pcolberg added the bug Something isn't working label Apr 22, 2022
@pcolberg pcolberg added this to the 2022.3 milestone Apr 22, 2022
@pcolberg pcolberg self-assigned this Apr 22, 2022
Currently it is too easy to fill the stringstream err_ss with an error
message but forget to assign the contents to the string err_str.

Reduce scope of stringstream err_ss to where it is used to make such
mistakes more obvious. In C++20, this may be replaced with std::format().

Signed-off-by: Peter Colberg <[email protected]>
Copy link
Contributor

@sherry-yuan sherry-yuan left a comment

Choose a reason for hiding this comment

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

Thanks Peter, LGTM!

Copy link
Contributor

@zibaiwan zibaiwan left a comment

Choose a reason for hiding this comment

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

Thanks Peter! LGTM!

@pcolberg pcolberg merged commit a446b30 into intel:main Apr 25, 2022
@pcolberg pcolberg deleted the stringstream branch April 25, 2022 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants