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

disk bucket: refactor storage get/get_mut fns #30985

Merged
merged 4 commits into from
Mar 30, 2023

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Mar 30, 2023

Problem

  1. There is duplicate code in the get/get_mut family of functions on bucket_storage
  2. These functions do not have safety checks for size/alignment

Summary of Changes

In commit 1, refactor the impl of the get/get_mut functions
In commit 2, move the functions within the file (no code changes)
In commit 3, remove get_from_parts functions, now that they are unused
In commit 4, fixup the impl from commit 1 😅

@brooksprumo brooksprumo marked this pull request as ready for review March 30, 2023 15:33
Copy link
Contributor

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov
Copy link

codecov bot commented Mar 30, 2023

Codecov Report

Merging #30985 (c188306) into master (38e054f) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #30985   +/-   ##
=======================================
  Coverage    81.5%    81.5%           
=======================================
  Files         728      728           
  Lines      205427   205427           
=======================================
+ Hits       167511   167526   +15     
+ Misses      37916    37901   -15     

@brooksprumo brooksprumo merged commit eb47e44 into solana-labs:master Mar 30, 2023
@brooksprumo brooksprumo deleted the bucket-map/get_mut branch March 30, 2023 16:38
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