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

Avoid potential corruption during unpack of static_variant #250

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

abitmore
Copy link
Member

@abitmore abitmore commented Oct 1, 2023

NOTE: this PR was merged but later reverted in #253, and #254 is to redo it.


Before this change, when sv.set_which(w.value) is already done and the following sv.visit(...) throws, the staic_variant object passed into the function may be updated to something incomplete and may cause unintended problems.

Although usually we don't reuse static_variant variables, it's not guaranteed.

This does lead to additional data copy.

Reference: https://gitlab.syncad.com/hive/hive/-/merge_requests/1049

@abitmore abitmore added this to the core release 6.2.0 or 7.1.0 milestone Oct 1, 2023
@abitmore abitmore force-pushed the update-unpack-static-variant branch 4 times, most recently from 063995b to ad82470 Compare October 1, 2023 09:13
@abitmore abitmore force-pushed the update-unpack-static-variant branch from ad82470 to 2d73734 Compare October 1, 2023 09:53
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@abitmore abitmore merged commit 9060dbc into master Feb 6, 2024
10 checks passed
@abitmore abitmore deleted the update-unpack-static-variant branch February 6, 2024 16:12
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.

1 participant