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.

Sign up for GitHub

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

Jump to bottom

Add sys module to mc-sgx-sync #39

Merged
merged 3 commits into from
Jan 30, 2023
Merged

Add sys module to mc-sgx-sync #39

merged 3 commits into from
Jan 30, 2023

Conversation

nick-mobilecoin
Copy link
Collaborator

The sys module contains a Mutex implementation for use in SGX enclaves

This was referenced Jan 17, 2023
Merged
Merged
Merged
@github-actions github-actions bot requested a review from varsha888 January 17, 2023 19:37
@nick-mobilecoin nick-mobilecoin mentioned this pull request Jan 17, 2023
Closed
@nick-mobilecoin nick-mobilecoin mentioned this pull request Jan 17, 2023
Merged
@github-actions github-actions bot added the size/M Medium-sized PRs label Jan 17, 2023
@nick-mobilecoin nick-mobilecoin mentioned this pull request Jan 17, 2023
Merged
@github-actions github-actions bot added the rust Pull requests that update rust code label Jan 17, 2023
This was referenced Jan 17, 2023
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@nick-mobilecoin
Copy link
Collaborator Author

nick-mobilecoin commented Jan 17, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

nick-mobilecoin
nick-mobilecoin commented Jan 17, 2023
View reviewed changes
sync/Cargo.toml Outdated Show resolved Hide resolved
@nick-mobilecoin nick-mobilecoin mentioned this pull request Jan 17, 2023
Merged
This was referenced Jan 23, 2023
Merged
Closed
@jcape jcape requested a review from samdealy January 24, 2023 18:20
@nick-mobilecoin nick-mobilecoin mentioned this pull request Jan 24, 2023
Merged
This was referenced Jan 25, 2023
Merged
Merged
Merged
jcape
jcape approved these changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@jcape jcape left a comment

Choose a reason for hiding this comment

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

The code here generally seems reasonable, but is the intent to put the poison state into this structure in a later PR?

sync/src/sys/mod.rs Outdated Show resolved Hide resolved
@nick-mobilecoin
Copy link
Collaborator Author

The code here generally seems reasonable, but is the intent to put the poison state into this structure in a later PR?

Poison state happens in std::sync::mutex.rs, which comes in in #40

@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2023 •
edited
Loading

Codecov Report

Merging #39 (329ce0a) into main (9f2836e) will increase coverage by 23.67%.
The diff coverage is n/a.

❗ Current head 329ce0a differs from pull request most recent head 0947923. Consider uploading reports for the commit 0947923 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff             @@
##             main      #39       +/-   ##
===========================================
+ Coverage   68.23%   91.91%   +23.67%     
===========================================
  Files           9        7        -2     
  Lines         318      235       -83     
===========================================
- Hits          217      216        -1     
+ Misses        101       19       -82
Impacted Files Coverage Δ
sync/src/lib.rs
sync/src/poison.rs

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

Base automatically changed from nick/poison to main January 26, 2023 17:37
This was referenced Jan 27, 2023
Merged
Merged
@nick-mobilecoin
Add sys module to mc-sgx-sync
6b0bae9 
The sys module contains a `Mutex` implementation for use in SGX enclaves
@nick-mobilecoin
Update to mainline mc-sgx-tstdc::Mutex interface
c957f74 
Previously the `mc-sgx-tstdc::Mutex::try_lock()` returned `Error::Busy`
for the lock being in use. Now `mc-sgx-tstdc::Mutex::try_lock()` returns
a boolean indicating if the lock was obtained and an `Err()` when there
is unexpected behavior.
@nick-mobilecoin
Move sync/src/sys/mod.rs to sync/src/sys.rs
0947923 
Update the root module for `sync/src/sys` to match newer rust file
naming convention,
https://doc.rust-lang.org/reference/items/modules.html#module-source-filenames
@nick-mobilecoin
Copy link
Collaborator Author

@varsha888 bumping for re-review due to restack

@nick-mobilecoin nick-mobilecoin mentioned this pull request Jan 30, 2023
Merged
@nick-mobilecoin
Copy link
Collaborator Author

nick-mobilecoin commented Jan 30, 2023 •
edited
Loading

✅ This pull request merged successfully as part of a Graphite job
Stack job ID: rnb0zcLhVk2UL31G5ihD.
See details on graphite.dev

@nick-mobilecoin nick-mobilecoin merged commit 89336a4 into main Jan 30, 2023
@nick-mobilecoin nick-mobilecoin deleted the nick/sys-mutex branch January 30, 2023 23:47
@nick-mobilecoin nick-mobilecoin mentioned this pull request Jan 31, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@varsha888 varsha888 varsha888 approved these changes

@samdealy samdealy samdealy approved these changes

@jcape jcape jcape approved these changes

Assignees

@nick-mobilecoin nick-mobilecoin

Labels
rust Pull requests that update rust code size/M Medium-sized PRs
Projects
Blockchain Core
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nick-mobilecoin @codecov-commenter @jcape @samdealy @varsha888