uefi/fs: add path and pathbuf abstraction

[phip1611]

I finally had time to work on the promised (#472 / #747 ) path and pathbuf abstraction for the uefi::fs module.

Once this is merged and the other stuff in the tracking issue is solved, I think it would be a good time for a new release :)

Status

• path and pathbuf abstraction that works as good as possible with CStr16 and CString16 and are close to the API of std::path
• fix the open todo in code (just a small thingy)
• use the new abstractions in the FileSystem abstraction
• update the changelog

API Design

For simplicity, there are no . or .. components in a path allowed. However, if someone ever needs this, this can be part of a future MR. However, I don't see why one would ever want this. There is no present working directory in a UEFI environment..

Open Questions

Users will not be able to pass in str and String.. but they can do something like: fs::write(cstr16!("foo\\bar")) and I think this is sufficient and convenient enough. Do you agree?

Some other Remarks

The Cartesian product of all convenient trait implementations ({AsRef, Borrow, From} x {Path, PathBuf, CStr16, CString16, str, String}) is quite challenging. I decided that there is only a good interoperability between {Path, PathBuf} x {CStr16, CString16} and {str, String} x {CStr16, CString16}.

Checklist

• Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
• Update the changelog (if necessary)

[phip1611]

The whole fs module was much more work than I expected 😀 However, feel free to have a look and leave some remarks, @nicholasbishop.

[phip1611]

I thought that there is a default implementation for that by the core lib. Apparently, there is not.

[nicholasbishop]

Users will not be able to pass in str and String.. but they can do something like: fs::write(cstr16!("foo\bar")) and I think this is sufficient and convenient enough. Do you agree?

Yep I think that's fine

[nicholasbishop]

I'm not sure if the SEPARATOR_STR and SEPARATOR_CSTR16 constants are needed. (Commented elsewhere about converting a use of SEPARATOR_CSTR16 -> SEPARATOR_C16). I know std has https://doc.rust-lang.org/std/path/constant.MAIN_SEPARATOR_STR.html, but I can't think of why one would use that instead of the char one.

[phip1611]

I also do not have a strong opinion here. Probably we can drop them, yes

[nicholasbishop]

Is there a bug here? I would have expected \\a instead of just a.

[phip1611]

This is intended and is documented on the Components struct. I do not specifically handle the root component as a dedicated path component, as all paths are absolute, always (as reflected by the module documentation). We do not have a present working directory here. Because of this simplification, Components doesn't need to be an enum but can be a struct.

Hence, "\a\b" and "a\b" are both absolute paths starting at root, in my abstraction.

[nicholasbishop]

Ah when reading the docs I missed this implication. The docs emphasize that in UEFI there's no CWD and no support for ./.., but to me that doesn't indicate relative paths aren't useful. For example:

fn get_boot_path(sub_path: &Path) -> PathBuf {
    let boot_path = Path::new(cstr16!(r"\efi\boot"));
    boot_path.join(sub_path);
}

get_boot_path(Path::new(cstr16!(r"somesubdir\somefile")));

There's no direct use of ./.. here, but relative paths are still needed. As you say in the PR description, this could be added later, which I think is fine. I do wonder though, if \a is the same as a, shouldn't Path have a custom eq impl so that the comparison reflects that?

[phip1611]

Ah, yeah, I see. Currently, there is no .join() method on a Path/PathBuf.

A Custom PartialEq-impl sounds reasonable

[phip1611]

Are we good to go now?

[nicholasbishop]

Lgtm!