Skip to content

Add support for the dup() syscall#1071

Merged
stevenengler merged 1 commit intoshadow:devfrom
stevenengler:dup
Jan 28, 2021
Merged

Add support for the dup() syscall#1071
stevenengler merged 1 commit intoshadow:devfrom
stevenengler:dup

Conversation

@stevenengler
Copy link
Copy Markdown
Contributor

Only works for the Rust descriptors, not the C descriptors.

@stevenengler stevenengler added Type: Enhancement New functionality or improved design Component: Main Composing the core Shadow executable labels Jan 25, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 25, 2021

Codecov Report

Merging #1071 (26fc4c4) into dev (043a506) will decrease coverage by 0.17%.
The diff coverage is 40.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1071      +/-   ##
==========================================
- Coverage   55.12%   54.95%   -0.18%     
==========================================
  Files         128      128              
  Lines       19335    19341       +6     
  Branches     4612     4612              
==========================================
- Hits        10659    10629      -30     
- Misses       5918     5958      +40     
+ Partials     2758     2754       -4     
Flag Coverage Δ
tests 54.95% <40.00%> (-0.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/main/host/syscall/unistd.c 43.75% <0.00%> (-0.76%) ⬇️
src/main/host/syscall_handler.c 53.80% <100.00%> (+0.47%) ⬆️
src/shim/preload_syscalls.c 48.01% <100.00%> (+0.23%) ⬆️
src/main/utility/utility.c 24.87% <0.00%> (-6.83%) ⬇️
src/support/logger/rust_bindings/src/lib.rs 37.18% <0.00%> (-1.60%) ⬇️
src/main/host/thread_ptrace.c 65.74% <0.00%> (-0.88%) ⬇️
src/main/core/slave.c 68.30% <0.00%> (-0.66%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 043a506...0975fe3. Read the comment docs.

dup_helper(sys, args, fd)
}

pub fn dup_helper(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I missed this before, but I think we want to constrain the proliferation of the c:: autogen'd types as much as we can, right? That way as more code gets migrated to Rust, it can just call the "rusty" implementation methods with native rust types, dropping the c:: types altogether. I think that'll let us consolidate more of the unsafe blocks into conversion functions as well, which again can go away once we no longer need the corresponding c:: types.

e.g. here I think it'd be preferable to have the extern "C" wrappers convert to/from rust "native" types. I think we'd need to add a SysCallHandler wrapper type (e.g. somewhat like we have for Thread, though it can be a bit simpler since we don't have "subclasses" of SysCallHandler). Also SysCallReturn could be added to syscall_types.rs, and probably some more...

I'm ok with punting on it for this PR but we should decide whether that's a direction we prefer before implementing a lot more rust handlers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I definitely agree that some of the Rust syscall handler code should be reorganized, and there are a few other things I want to change as well. I didn't want to add too much code in that previous PR, but plan to improve this soon when I start working on more descriptor types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Main Composing the core Shadow executable Type: Enhancement New functionality or improved design

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants