Conversation
sporksmith
commented
Sep 15, 2020
- Handle PROT_NONE in mmap
- Handle mprotect syscall
- Handle gettid syscall
- Add and enable pthread_create detached thread test
Codecov Report
@@ Coverage Diff @@
## dev #986 +/- ##
==========================================
+ Coverage 52.80% 53.02% +0.22%
==========================================
Files 125 126 +1
Lines 18146 18187 +41
Branches 4320 4329 +9
==========================================
+ Hits 9582 9644 +62
+ Misses 5877 5845 -32
- Partials 2687 2698 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
robgjansen
left a comment
There was a problem hiding this comment.
LGTM! I'm really falling behind w.r.t. Rust though, so you might wanna get @stevenengler to look over the Rust bits.
|
@stevenengler could you take a look at the rust? |
stevenengler
left a comment
There was a problem hiding this comment.
I don't really follow what's happening in handle_mprotect(), but the rust code generally looks good to me.
src/main/host/memory_manager.rs
Outdated
| size: usize, | ||
| prot: i32, | ||
| ) -> c::SysCallReg { | ||
| let memory_manager = &mut *memory_manager; |
There was a problem hiding this comment.
Check for a null pointer here?
There was a problem hiding this comment.
Good point. Done throughout.
|
Oh yeah, I should have included the general comment that I would have liked some additional comments to explain what was going on in some of the Rust code. Seems like that could be helpful for Steven too. |
|
Maybe for another PR, but I think the memory manager could use more top-level comments. For example, what the memory manager does and what it's responsible for, what do functions like |
|
I have PR #987 q'd up that builds on this one, switching from println to the logging macros. I'll make a pass on the comments in that PR is as well. |