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

Add PathInitializer to mc-sgx-dcap-ql #166

Merged
merged 3 commits into from
Oct 20, 2022
Merged

Conversation

nick-mobilecoin
Copy link
Collaborator

Add a PathInitializer struct to mc-sgx-dcap-ql to ensure that the
necessary DCAP quote library paths are set prior to making quote library
calls. As well as ensuring that the paths are only initialized once.

@github-actions github-actions bot added the size/L PRs with more than 500 lines of changes label Oct 12, 2022
Copy link
Contributor

@NotGyro NotGyro left a comment

Choose a reason for hiding this comment

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

This looks good.

@nick-mobilecoin nick-mobilecoin marked this pull request as draft October 14, 2022 18:06
@nick-mobilecoin nick-mobilecoin marked this pull request as ready for review October 14, 2022 18:24
@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2022

Codecov Report

Merging #166 (3100487) into feature/sealing_parameter_validation (9cc6746) will increase coverage by 0.46%.
The diff coverage is 94.77%.

@@                           Coverage Diff                            @@
##           feature/sealing_parameter_validation     #166      +/-   ##
========================================================================
+ Coverage                                 86.67%   87.13%   +0.46%     
========================================================================
  Files                                        41       41              
  Lines                                      2251     2371     +120     
========================================================================
+ Hits                                       1951     2066     +115     
- Misses                                      300      305       +5     
Impacted Files Coverage Δ
dcap/ql/src/quote3.rs 0.00% <0.00%> (ø)
dcap/ql/src/quote_enclave.rs 94.56% <95.34%> (+1.91%) ⬆️
dcap/ql/src/lib.rs 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@nick-mobilecoin nick-mobilecoin force-pushed the feature/sealing_parameter_validation branch from 15b47a1 to 9cc6746 Compare October 14, 2022 22:10
@nick-mobilecoin nick-mobilecoin force-pushed the feature/sealing_parameter_validation branch from 9cc6746 to c89b535 Compare October 17, 2022 20:59
Base automatically changed from feature/sealing_parameter_validation to main October 17, 2022 21:12
Add a `PathInitializer` struct to `mc-sgx-dcap-ql` to ensure that the
necessary DCAP quote library paths are set prior to making quote library
calls.  As well as ensuring that the paths are only initialized once.
Previously `PATH_INITIALIZER` was a OneCell, this required the use of
undefined behavior to sufficiently test. Now `PATH_INITIALIZER` uses a
Lazy Mutex which can be consistently reset for testing purposes
Previously `mc-sgx-ql::PathInitializer::with_paths()` used the same
generic type for all the pathlike parameters. This had the side effect that all four
paths had to be the same type. Now each pathlike parameter has it's own
type.
Copy link
Contributor

@remoun remoun left a comment

Choose a reason for hiding this comment

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

LGTM

dcap/ql/src/quote_enclave.rs Show resolved Hide resolved
dcap/ql/src/quote_enclave.rs Show resolved Hide resolved
@nick-mobilecoin nick-mobilecoin merged commit 8410826 into main Oct 20, 2022
@nick-mobilecoin nick-mobilecoin deleted the feature/ql_paths_once branch October 20, 2022 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L PRs with more than 500 lines of changes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants