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

feat: lint fuzz projects in CI #283

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Bogay
Copy link

@Bogay Bogay commented Jan 24, 2025

This PR adds steps to run cargo fmt and cargo clippy to fuzz_write and fuzz_read, also fixed some issues found by fmt and clippy.

Bogay added 4 commits January 20, 2025 19:43
which makes current nightly clippy fail.
it seems that they depends on cargo-afl, so we need to use
`cargo afl clippy` instead of `cargo clippy`.
In this commit I replaced `self.path.join("/")` with
`self.path.join("")` because I think the expected result should be
the original path with trailing "/".

see: https://rust-lang.github.io/rust-clippy/master/index.html#join_absolute_paths
BasicFileOperation::WriteDirectory(_) => Some(self.path.join("/")),
BasicFileOperation::WriteDirectory(_) => Some(self.path.join("")),
Copy link
Author

Choose a reason for hiding this comment

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

I am not sure whether this is intened. I guess in this line we want to return a path with trailing /, so I replaced the params with "".

see: https://rust-lang.github.io/rust-clippy/master/index.html#join_absolute_paths

Copy link
Member

@Pr0methean Pr0methean Jan 26, 2025

Choose a reason for hiding this comment

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

This probably isn't the correct behavior either. What's the correct way to append a / to a path? That's how we detect that it is, in fact, intended to be a directory and not a normal file. The documentation indicates, if I'm reading it correctly, that self.path.join("") doesn't do that and is instead a no-op. Maybe we need to append the / at some point while it's still a string-like type?

Copy link
Author

Choose a reason for hiding this comment

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

The documentation indicates, if I'm reading it correctly, that self.path.join("") doesn't do that and is instead a no-op.

I think self.path.join("") appends a / at the end of path. You can verify that with following code:

use std::path::PathBuf;

fn main() {
    let original_path = PathBuf::from("/home/user");
    let joined_path = original_path.join("");

    println!("Original path: {:?}", original_path);
    println!("Joined path: {:?}", joined_path);
    assert_eq!(joined_path, PathBuf::from("/home/user/"));
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=37109e0971e2e92e6ef255d46d3f8eeb

@Bogay
Copy link
Author

Bogay commented Jan 24, 2025

I found 4cebeba in this PR is also fixed by #279 . Maybe it can be merged first and then I can rebase later.

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.

2 participants