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

get_disks() doesn't return all mounted disks on Mac OS #375

Closed
hippietrail opened this issue Dec 9, 2020 · 17 comments · Fixed by #377
Closed

get_disks() doesn't return all mounted disks on Mac OS #375

hippietrail opened this issue Dec 9, 2020 · 17 comments · Fixed by #377

Comments

@hippietrail
Copy link

hippietrail commented Dec 9, 2020

I've just started learning Rust. I'm on a 2013 MacBook Air running macOS Sierra and rustc 1.48.0

I modified the sysinfo example to just print out the disks in the system. My boot disk is an external USB 3.0 SSD. (The internal SSD has been missing since I got the laptop.) Running the code with just the boot disk reports just the boot disk. Attaching my USB 3.0 HD with several partitions including Mac, FAT, and NTFS; still only the boot disk is listed. I added more refresh_* calls but it made no difference. Code:

use sysinfo::SystemExt;

fn main() {
    let mut system = sysinfo::System::new_all();

    // First we update all information of our system struct.
    system.refresh_all();

    // And then all disks' information:
    for disk in system.get_disks() {
        println!("{:?}", disk);
    }
}

Output:

> Executing task: cargo run --package disklist --bin disklist <

   Compiling disklist v0.1.0 (/Users/hippietrail/disklist)
    Finished dev [unoptimized + debuginfo] target(s) in 2.32s
     Running `target/debug/disklist`

UPDATE

It turns out I had my .toml set up to use version "0.3" of sysinfo, which I must've got from Googling. I changed it to "0.15" to get the latest version. Instead of fixing the problem it now prints out nothing at all. Not even my boot disk is listed.

UPDATE 2

It looks like I had code that was still using System::new() from the 0.3 example so I changed it to System::new_all() to match the 0.15 version. Now on my Mac the boot disk shows up again but neither of two different USB hard drives, nor any of their partitions.

On my Windows 10 machine there's a different problem. The internal SSDs show up and the external USB HDs that don't show up on the Mac do show up, but my SDHC card does not show up.

@GuillaumeGomez
Copy link
Owner

That's intriguing. I'll need to check what's going there.

@hippietrail
Copy link
Author

That's intriguing. I'll need to check what's going there.

Thanks Guillaume. I updated the report by fixing a problem in the code that was mixing 0.3 and 0.15 versions of the example code and tested under Windows 10, which has a different problem.

@hippietrail
Copy link
Author

In utils.rs lines 41 & 42 ...

        if res < 0 || and(buf.st_mode.into(), S_IFMT.into()) != S_IFLNK.into() {
            PathBuf::new()

It seems you are only considering an entry in /Volumes to be a mountpoint if it is a symlink. On my system my boot drive is a symlink but the other drives are directories. So unless this check is here for a specific reason not apparent to me it seems to be wrong to filter out non-symlinks.

@GuillaumeGomez
Copy link
Owner

I only have my own Mac to check that so if you're telling me that it works in your case, then we can simply remove this check.

@hippietrail
Copy link
Author

My friend has also checked it on his Mac and confirms:

I have a usb drive, a network drive and a timemachine snapshot
... can see only one softlink

@GuillaumeGomez
Copy link
Owner

Testing with a USB key is a good idea! I'll give it a try.

@hippietrail
Copy link
Author

hippietrail commented Dec 11, 2020

It seems that on Mac a USB key is treated identically to USB hard drive or SSD but on Windows USB hard drives and SSDs are included but USB keys are rejected. Technically a separate bug but as the crate currently doesn't list any external drives on Mac I won't open a new issue yet.

@GuillaumeGomez
Copy link
Owner

So a good example why I didn't take into account folders. On my computer I have in my /Volumes:

lrwxr-xr-x   1 root     wheel      1 Jul 11 19:14 Macintosh HD -> /
drwxr-xr-x   3 root     wheel     96 Jul 11 13:43 Recovery
drwxr-xr-x  10 imperio  staff    408 Sep  7  2019 Steam

Obviously, Recovery and Steam aren't hard drives. Now something interesting happens with USB drives:

drwxrwxrwx@  1 imperio  staff  16384 Dec 11 20:47 Untitled
drwxrwxrwx@  1 imperio  staff  16384 Aug 26 12:43 key

As you can see, there is a @ at the end, so I need to check how to get this info.

@GuillaumeGomez
Copy link
Owner

So after spending quite some time on this, the only thing I could find for the moment was getFileSystemInfoForPath, but I wasn't able to use it in rust because I can't find the C symbol which would allow me to use it. The code I added to try it out is (you'll need to uncomment a few things in mac/ffi.rs too: CFStringGetCharactersPtr, CFArrayGetCount, CFArrayGetValueAtIndex, CFStringRef, CFArrayRef, CFIndex):

#[repr(C)]
pub struct NSWorkspace {
    __private: c_void,
}

extern "C" {
    fn NSWorkspaceSharedWorkspace() -> *mut NSWorkspace;
    fn NSWorkspaceMountedLocalVolumePaths(ws: *mut NSWorkspace) -> *mut c_void;
    fn getFileSystemInfoForPath(
        ws: *mut NSWorkspace,
        path: ffi::CFStringRef,
        isRemovable: *mut ffi::Boolean,
        isWritable: *mut ffi::Boolean,
        isUnmountable: *mut ffi::Boolean,
        description: ffi::CFStringRef,
        type_: ffi::CFStringRef,
    ) -> ffi::Boolean;
}

pub(crate) fn get_disks() -> Vec<Disk> {
    unsafe {
        let ws = NSWorkspaceSharedWorkspace();
        let vols = NSWorkspaceMountedLocalVolumePaths(ws);

        for x in 0..ffi::CFArrayGetCount(vols as _) {
            let path = ffi::CFArrayGetValueAtIndex(vols as _, x);
            let mut isRemovable = 0;
            let mut isWritable = 0;
            let mut isUnmountable = 0;
            let description = ffi::CFStringCreateWithCStringNoCopy(
                ws,
                ptr::null_mut(),
                [0].as_ptr() as *const c_char,
                ffi::kCFStringEncodingMacRoman,
                ffi::kCFAllocatorNull as *mut c_void,
            );
            let res = getFileSystemInfoForPath(path as _, &mut isRemovable, &mut isWritable, &mut isUnmountable, description, ::std::ptr::null_mut());
            if res == 0 {
                continue;
            }
            let tmp = std::ffi::CStr::from_ptr(ffi::CFStringGetCharactersPtr(path as _) as _);
            let tmp_d = std::ffi::CStr::from_ptr(ffi::CFStringGetCharactersPtr(description)as _);
            println!("==> {:?} {:?} w: {:?} r: {:?} u: {:?}", tmp.to_str().unwrap(), tmp_d.to_str().unwrap(), isWritable, isRemovable, isUnmountable);
        }
    }
    vec![]
}

If anyone is interested into trying to fix more things...

@hippietrail
Copy link
Author

hippietrail commented Dec 11, 2020

The @ means there are extended attributes. You can see them by doing for example ls -l@

drwxrwxrwx@ 1 hippietrail  staff  131072  1 Jan  1980 Backup Plus
	com.apple.FinderInfo	    32 
drwxrwxrwx@ 1 hippietrail  staff   16384 12 Dec 08:58 CRUZERFAT8
	com.apple.filesystems.msdosfs.volume_id	     4 
lrwxr-xr-x  1 root         wheel       1 10 Dec 18:08 Mac USB SSD -> /
drwxrwxrwx@ 1 hippietrail  staff   32768 11 Dec 20:47 SANDISKµ16
	com.apple.filesystems.msdosfs.volume_id	     4 

The first is a hard drive with one MS partition, the second is a USB stick, the third is my boot drive, and the last is a microSD card attached via a USB adapter. Except for the boot drive, they are all connected via a $5 USB 2 hub.

The SD card and the USB stick are not partitioned. So the only pattern I can see is that unpartitioned drive with MS filesystems get com.apple.filesystems.msdosfs.volume_id and partitioned ones get com.apple.FinderInfo.

I'm starting to think that /Volumes/ is not the right place to start. The nodeJS module "drivelist" uses a different approach. You can see its Objective C code here.

I have to admit that so far I'm not very good at reading mscOS API code or Objective C though. It uses the Disk Arbitration system.

@hippietrail
Copy link
Author

Another way to fool the current code is to make a random symlink in the /Volumes directory:

sudo ln -s ~ /Volumes/a-link

Current get_disks() on Mac will consider you to have a disk named a-link mounted at /Users/yourusername

@hippietrail
Copy link
Author

hippietrail commented Dec 14, 2020

Here's some proof of concept code I managed to get working in very terrible Swift. It doesn't seem to have false positives or false negatives for anything that might be in /Volumes and it should hopefully filter hd/ssd vs usb stick/sdcard to match the Windows code.

import Cocoa
import DiskArbitration

if let session = DASessionCreate(kCFAllocatorDefault) {
    let mountedVolumeURLs = FileManager.default.mountedVolumeURLs(includingResourceValuesForKeys: nil)!
    for volumeURL in mountedVolumeURLs {
        if let disk = DADiskCreateFromVolumePath(kCFAllocatorDefault, session, volumeURL as CFURL),
            let bsdName = DADiskGetBSDName(disk) {
            let bsdString = String(cString : bsdName)
            print(volumeURL.path, bsdString)
            
            if let descDict = DADiskCopyDescription(disk) as? [String: CFTypeRef] {
                let removable : Bool, ejectable : Bool
                if let val = descDict["DAMediaRemovable"] as? Bool {
                    removable = val
                    if let val = descDict["DAMediaEjectable"] as? Bool {
                        ejectable = val

                        var type = ""
                        
                        type += removable || ejectable ? "USB stick, SD card, etc" ? "hard drive, SSD, etc";
                        
                        type += " ("
                        
                        type += removable ? "" : "not "
                        type += "removable"
                        type += ", "
                        type += ejectable ? "" : "not "
                        type += "ejectable"
                        
                        type += ")"

                        print(" ", type)
                    }
                }
            }
            print("\n")
        }
    }
}

@GuillaumeGomez
Copy link
Owner

Another way to fool the current code is to make a random symlink in the /Volumes directory:

sudo ln -s ~ /Volumes/a-link

Current get_disks() on Mac will consider you to have a disk named a-link mounted at /Users/yourusername

We definitely shouldn't do that, I'll try to find another way. ;)

@GuillaumeGomez
Copy link
Owner

@hippietrail Just for your information, I'm almost done rewriting the code in rust. My last issue is with mountedVolumeURLsIncludingResourceValuesForKeys (which is mountedVolumeURLs in your swift code). Still trying to find the good symbol for the linker.

@hippietrail
Copy link
Author

hippietrail commented Dec 26, 2020

I have a hunch that some things in Swift come from its runtime library that wrap or abstract around some system calls. I haven't found a Swift equivalent of The Rust Book yet though so I haven't learned much but I believe I ran into this problem when I was looking at app notifications. Perhaps it wraps an Objective C system call in a Swift dictionary/associative array?

Or maybe not. Here it seems to be used directly from ObjC.

@GuillaumeGomez
Copy link
Owner

Yes, that's why I mentioned mountedVolumeURLsIncludingResourceValuesForKeys. ;)

But the problem is that it's still a method from the NSFileManager object, and I need to get the default field from this NSFileManager to then be able to call the method. So much for simplicity I guess...

@GuillaumeGomez
Copy link
Owner

Since it's all about passing messages to emulate objects and methods, I might be able to have the NSFileManager in Rust directly thanks to https://github.com/apportable/Foundation/blob/master/System/Foundation/include/Foundation/NSFileManager.h

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 a pull request may close this issue.

2 participants