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

Fix build on Redox, and add stat to Redox feature #5502

Merged
merged 1 commit into from Dec 11, 2023

Conversation

ids1024
Copy link
Contributor

@ids1024 ids1024 commented Nov 6, 2023

No description provided.

@@ -1883,6 +1883,7 @@ fn copy_file(
Ok(())
}

#[allow(clippy::unnecessary_cast)]
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

do you know many occurrences of this happen in this function? If not too many, could you please move that into the function itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, looking at the function again, I see there's already a seperate case with an as u32 for FreeBSD / macOS / Android which are using u16. So Redox could be moved to that case. Though just using a cast with this #[allow] is simpler.

I wonder if it would be better to just not use these constants, and just use octal notation here? Their values should be standardized, and I think people may be more familiar with the octal values for Unix-style permissions than with the constant names.

@@ -148,9 +149,9 @@ pub fn parse_mode(mode: &str) -> Result<mode_t, String> {
let fperm = (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) as u32;

let result = if mode.chars().any(|c| c.is_ascii_digit()) {
parse_numeric(fperm, mode, true)
parse_numeric(fperm as _, mode, true)
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

please add a comment explaining the "as _"

@@ -137,6 +137,7 @@ fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) {
(srwx, pos)
}

#[allow(clippy::unnecessary_cast)]
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

same comment as above

@sylvestre
Copy link
Sponsor Contributor

LGTM
it would be great to have CI for this (test would be ideal but at least a job to do cross compilation)

@sylvestre sylvestre merged commit 62afd78 into uutils:main Dec 11, 2023
53 of 55 checks passed
@jackpot51
Copy link
Collaborator

I can work on CI soon. Sorry I haven't gotten to it yet.

@sylvestre
Copy link
Sponsor Contributor

no need to be sorry :)

@jackpot51
Copy link
Collaborator

WIP: #5642

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.

None yet

3 participants