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

df: fix output if input path is device name #3682

Merged
merged 1 commit into from
Jun 30, 2022

Conversation

cakebaker
Copy link
Contributor

Currently, when providing a device name as a path, you get some output like:

$ ./target/debug/coreutils df /dev/sda2 --out=source,target
Filesystem     Mounted on
dev            /dev

With this PR, the output is now the same as the output from GNU df:

$ ./target/debug/coreutils df /dev/sda2 --out=source,target
Filesystem     Mounted on
/dev/sda2      /boot

Fixes #3246

Copy link
Contributor

@anastygnome anastygnome left a comment

Choose a reason for hiding this comment

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

Good, but there's room for improvement :

find returns the first match (so index 0) or None, which we can then catch using the or_else clause.

This allows for further compile time optimisations ;)

Copy link
Contributor

@anastygnome anastygnome left a comment

Choose a reason for hiding this comment

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

Woops, didn't get the whole range.

@@ -67,6 +67,16 @@ where
} else {
path.as_ref().to_path_buf()
};

Copy link
Contributor

@anastygnome anastygnome Jun 29, 2022

Choose a reason for hiding this comment

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

Why not do something like this instead ?

    let maybe_mount_point: Option<&MountInfo> = mounts
        .iter()
        .find(|mi| mi.dev_name.eq(&path.to_string_lossy()));

    maybe_mount_point.or_else(|| {
        mounts
            .iter()
            .filter(|mi| path.starts_with(&mi.mount_dir))
            .max_by_key(|mi| mi.mount_dir.len())
    })
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion. This is indeed a much cleaner solution and I switched to it in the recent push.

@sylvestre sylvestre merged commit 63bf7db into uutils:main Jun 30, 2022
@cakebaker cakebaker deleted the ticket_3246 branch July 1, 2022 05:23
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.

df: df doesn't check if the input path is a filesystem
3 participants