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

Simplified syscall error #1949

Merged
merged 2 commits into from
May 20, 2023
Merged

Simplified syscall error #1949

merged 2 commits into from
May 20, 2023

Conversation

yihuaf
Copy link
Collaborator

@yihuaf yihuaf commented May 20, 2023

  • Refactored the syscall errors. There is no need to create an error for each syscall. Only the errno is what is required. Other context can be logged at the site of the errors.
  • Removed the UnifiedSyscallError. It was not well thought out at the moment.
  • Refactored close_range and mount_set_attr to use libc::syscall directly.

Signed-off-by: yihuaf <yihuaf@unkies.org>
Signed-off-by: yihuaf <yihuaf@unkies.org>
@codecov-commenter
Copy link

codecov-commenter commented May 20, 2023

Codecov Report

Merging #1949 (c45e5f2) into main (0d6b0d5) will decrease coverage by 0.06%.
The diff coverage is 14.28%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1949      +/-   ##
==========================================
- Coverage   64.88%   64.82%   -0.06%     
==========================================
  Files         127      127              
  Lines       14674    14659      -15     
==========================================
- Hits         9521     9503      -18     
- Misses       5153     5156       +3     

@yihuaf yihuaf requested review from utam0k and a team May 20, 2023 06:57
@yihuaf yihuaf self-assigned this May 20, 2023
@utam0k
Copy link
Member

utam0k commented May 20, 2023

Refactored the syscall errors. There is no need to create an error for each syscall. Only the errno is what is required. Other context can be logged at the site of the errors.

I agree with you! It would be wiser for them to see man than for us to give them the message from us.

@utam0k
Copy link
Member

utam0k commented May 20, 2023

I've been working on this area lately, and thiserror makes it easy to identify errors. Thanks.

@utam0k utam0k merged commit c5503d5 into youki-dev:main May 20, 2023
@yihuaf yihuaf deleted the yihuaf/syscall-error branch May 22, 2023 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants