Skip to content

Rename borrowing accessors#2623

Merged
sporksmith merged 2 commits intoshadow:mainfrom
sporksmith:rename-borrowing-accessors
Dec 20, 2022
Merged

Rename borrowing accessors#2623
sporksmith merged 2 commits intoshadow:mainfrom
sporksmith:rename-borrowing-accessors

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

Follow-up to @stevenengler's feedback on #2612.

  • Rename APIs that return borrow-counted objects
  • add #[track_caller] to borrowing APIs

This makes it clearer to the caller that they need to be careful to
avoid conflicting borrows when using such APIs.
This a borrow in one of these methods fails at runtime, this annotation
causes the panic to identify the caller, which is more likely where the
real issue is.
@sporksmith sporksmith self-assigned this Dec 20, 2022
@sporksmith sporksmith enabled auto-merge December 20, 2022 17:00
@github-actions github-actions bot added Component: Libraries Support functions like LD_PRELOAD and logging Component: Main Composing the core Shadow executable labels Dec 20, 2022
@sporksmith sporksmith disabled auto-merge December 20, 2022 17:00
@sporksmith sporksmith enabled auto-merge December 20, 2022 17:00
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 20, 2022

Codecov Report

Base: 67.54% // Head: 68.19% // Increases project coverage by +0.64% 🎉

Coverage data is based on head (bd643c1) compared to base (e150143).
Patch coverage: 84.76% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2623      +/-   ##
==========================================
+ Coverage   67.54%   68.19%   +0.64%     
==========================================
  Files         200      200              
  Lines       29509    29522      +13     
  Branches     5789     5791       +2     
==========================================
+ Hits        19933    20132     +199     
+ Misses       4993     4779     -214     
- Partials     4583     4611      +28     
Flag Coverage Δ
tests 68.19% <84.76%> (+0.64%) ⬆️

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

Impacted Files Coverage Δ
src/main/host/memory_manager/mod.rs 76.45% <ø> (ø)
src/main/host/syscall/handler/eventfd.rs 16.66% <0.00%> (+16.66%) ⬆️
src/main/host/syscall/handler/sysinfo.rs 91.30% <50.00%> (+0.39%) ⬆️
src/main/host/syscall/handler/ioctl.rs 57.14% <66.66%> (+17.14%) ⬆️
src/main/host/syscall/handler/sched.rs 44.77% <80.00%> (ø)
src/main/host/host.rs 81.43% <83.33%> (+0.73%) ⬆️
src/main/host/syscall/handler/time.rs 67.44% <83.33%> (+5.53%) ⬆️
src/main/host/process.rs 85.08% <86.48%> (+0.33%) ⬆️
src/main/host/syscall/handler/socket.rs 68.50% <87.50%> (+0.51%) ⬆️
src/main/core/worker.rs 82.55% <100.00%> (ø)
... and 27 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Copy Markdown
Member

@robgjansen robgjansen left a comment

Choose a reason for hiding this comment

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

Nit: I think you want the extended commit message of bd643c1 to start with "When" instead of "This".

@sporksmith sporksmith merged commit 1f71d77 into shadow:main Dec 20, 2022
@sporksmith sporksmith deleted the rename-borrowing-accessors branch December 20, 2022 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Libraries Support functions like LD_PRELOAD and logging Component: Main Composing the core Shadow executable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants