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

selinux: write xattr related codes. #2825

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Gekko0114
Copy link
Contributor

This is an experimental crate. I'll implement a selinux crate step-by-step.
In this PR, I implemented functions related to xattr.

ref: #2718 #2800

@Gekko0114 Gekko0114 marked this pull request as ready for review June 22, 2024 07:16
@Gekko0114
Copy link
Contributor Author

Hi @utam0k, @YJDoc2,
I've created this small PR adding the functions related to xattr. Could you review it?

@utam0k
Copy link
Member

utam0k commented Jun 25, 2024

I'll visit this PR in a couple of days. Please give me some time.

experiment/selinux/src/xattrs/xattr.rs Outdated Show resolved Hide resolved
let size = rfs::getxattr(fpath, attr, &mut buf)
.map_err(|e| XattrError::GetXattr(e.to_string()))?;
let mut value = String::from_utf8_lossy(&buf[..size]).into_owned();
if (!value.is_empty()) && (value.ends_with('\x00')) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the \x00 be checked and the buffer size adjusted if it isn't termination.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it. Is this implementation same as your intention?

@Gekko0114
Copy link
Contributor Author

Hi @utam0k, @YJDoc2,
Thanks for your comments and fixed them. Could you take a look again when you have time?

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.29%. Comparing base (46fb4ba) to head (3123107).
Report is 17 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2825      +/-   ##
==========================================
+ Coverage   66.27%   66.29%   +0.02%     
==========================================
  Files         131      131              
  Lines       16784    16794      +10     
==========================================
+ Hits        11123    11134      +11     
+ Misses       5661     5660       -1     

Copy link
Collaborator

@YJDoc2 YJDoc2 left a comment

Choose a reason for hiding this comment

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

Hey, a couple of questions, and the CI is failing due to unrelated issues, can you rebase on current main so CI will pass? Thanks!

experiment/selinux/src/selinux.rs Outdated Show resolved Hide resolved
experiment/selinux/src/xattrs/xattr.rs Outdated Show resolved Hide resolved
@YJDoc2
Copy link
Collaborator

YJDoc2 commented Jun 29, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

@utam0k should we remove the codecov bot for now? We are not enforcing the coverage value in CI, and also not maintaining a required threshold for it.

Signed-off-by: Hiroyuki Moriya <41197469+Gekko0114@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/experimental `/experimental`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants