Tracking Issue: High-level API filesystem usability

[RaitoBezarius]

This is a follow-up of #747, I have been playing around with it and I have a bit of wishlist (of course, would love to contribute directly some of them), but want to discuss with you.

1. FileSystemResult doesn't compose well with uefi::Result, maybe we should have a 4th variant UEFIError or something or transform easily any FileSystemResult into a uefi::Result, I don't really know what is the best but what I am seeing is that we need more powerful and user-friendly errors when writing FS code that mixes UEFI code and FS code.
2. Constructing paths: there's no join operations at the moment I believe.
3. Constructing OS paths: while we live in UEFI land, and we use CStr16 everywhere, sometimes, we would like to write UNIX or Windows paths in some file that we are preparing for boot, e.g. in lanzaboote, we would like to assemble a CPIO archive, the paths are therefore UTF-8 UNIX paths. For this, I wish I had std::path::Path, but I don't :-). It would be interesting to see if we can find an alloc solution for that (maybe it's a Rust upstream issue).
4. is_ascii is a missing on CStr16 I think (same for is_utf8), it would be useful to have it for many FS work (where I want to exclude any weird filenames).
5. Weird impedence mismatch between fs.open_volume and opening a filesystem with SimpleFilesystem, I can never get a Directory this way, how is it envisioned to get a handle on the root directory in the high level API?

I think this is all I can think of right now. (feel free to rename the issues or whatever.)

[RaitoBezarius]

To give an interesting example, how would you complete this:

/// Returns the "default" drop-in directory if it exists.
/// This will be in general $loaded_image_path.extra/
pub fn get_default_dropin_directory(boot_services: &BootServices) -> Option<Directory> {
    if let Some(image_file_path) = boot_services
        .open_protocol_exclusive::<LoadedImage>(boot_services.image_handle())
        .map(|loaded_image| loaded_image.file_path())? {
        // We could use LoadedImageDevicePath to get the full device path
        // and perform replacement of the last node before END_ENTIRE
        // by another node containing the filename + .extra
        // But this is as much tedious as performing a conversion to string
        // then opening the root directory and finding the new directory.
        let image_file_string = image_file_path.to_string(boot_services, DisplayOnly(false), AllowShortcuts(false))?
            .expect("Failed to obtain the string representation of the loaded image file path; not enough memory?");
        let target_directory: CString16 = format!("{}.extra", image_file_string);
        let fs = uefi::fs::FileSystem::new(boot_services.get_image_file_system(
                boot_services.image_handle()
        )?);

        if let Ok(info) = fs.metadata(target_directory.into()) {
            if info.is_directory() {
            }
        }
    }

    None
}

Given that fs.open is private, there's no way for me to open this info without re-opening the SFS, opening the root directory, opening the path and into-ing into that Directory.

I think having a way to go from high level to low level would be very appreciable because I don't want to move around listing of directories, only a file handle.

[nicholasbishop]

Hi, thanks for filing this. Here are my initial thoughts:

1. For composing errors, it might be best to make your own error enum? E.g.

    #[derive(Debug)]
    enum GetDropinDirectoryError {
        LoadedImageError(uefi::Error),
        NoFilePath,
        Oom,
        DevicePathToTextError(uefi::proto::device_path::DevicePathToTextError),
        InvalidPath(uefi::data_types::FromStrError),
        FileSystemError(uefi::fs::FileSystemResult),
    }

   Or potentially use thiserror.
   Or you could maybe use Box<dyn Error> or the anyhow crate, if you are OK with unstable features to enable the Error trait in core.
2. Adding a join method for paths sounds good, I think this was discussed during the initial implementation and left as future work.
3. I think adding is_ascii to Char16 and CStr16 (which implicitly provides it for CString16 as well) should be uncontroversial if you want to open a PR with just that part we can get it reviewed quickly.
   • In your WIP PR there's also a new SliceLikeChar16 trait, I'm not sure if that's necessary -- would want to see stronger motivation for it
   • I don't think is_utf8 would make sense, since CStr16 is inherently UCS-2 and can always be losslessly converted to utf8.
4. Re std::path::Path, there is work underway to support std for the UEFI targets: Tracking Issue for uefi-std rust-lang/rust#100499. If you use a nightly compiler that could already be an option for you.
5. uefi::fs::FileSystem is intended to mostly mirror std::fs; both of them operate on paths rather than file handles. So for your get-dropin-dir example, I think you could either have it just return target_directory as a path, or you could switch to using the lower-level SimpleFileSystem API. The latter requires keeping the protocol open for the duration of your use, otherwise the Directory handle would become invalid.

[phip1611]

Yes, as @nicholasbishop told, our goal for uefi::fs::FileSystem is to match std::fs as closely as possible. The same is true for uefi::fs::Path.

1. I'm here with Nicholas. We should not convert a high-level FileSystem error to an low-level uefi-error. This feels wrong. Working with the Error trait might be a reasonable solution, or just use a new enum covering all error variants.
2. (no additional remarks)
3. (no additional remarks)
4. (no additional remarks)
5. (no additional remarks)

[RaitoBezarius]

Opened #1008 for the is_ascii stuff, closed the SliceLike stuff, I am fine with inlining the iteration if you prefer, it means that is_ascii is impossible on &[Char16] though, but it's fine I guess.