EqStrUntilNul trait to compare Rust strings (str, String) against CStr16 and CString16

[phip1611]

I cleaned up and improved the code of .eq()-implementations for CStr16 and CString16. Now it is easy to compare &str and String from the standard library with UEFI strings.

Let me know what you think.

According to the discussion, I changed the PR. Now there is a common trait which allows to compare Rust strings (str, String) against CStr16 and CString16.

PS: If it's okay for you, I'd be happy to support you as a maintainer/developer in uefi-rs as I'm submitting stuff to uefi-rs anyway from time to time. Just let me know if you need support

[nicholasbishop]

Thanks for the PR! Looks like there is one failing lint:

error: this creates an owned instance just for comparison
Error:    --> uefi-test-runner/src/proto/media/known_disk.rs:174:12
    |
174 |         if fs_info.volume_label().to_string() == "MbrTestDisk" {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `fs_info.volume_label()`
    |
    = note: `-D clippy::cmp-owned` implied by `-D warnings`

I have a bit of hesitation on allowing CStr16 == str though:

1. It differs from the standard library; core::ffi::CStr == str isn't currently allowed. Playground. (I did a quick search for rustlang issues to see if such a comparison had been discussed, but didn't find anything immediately.)
2. Although str doesn't require a trailing null, it doesn't disallow it either. "abc\0" is a legal string. Playground. So some edge cases have to be considered. Is a CStr16 containing "abc" equal to both a str containing "abc" and a str containing "abc\0"? What about a str containing "abc\0xyz"?

That said, having the comparison does feel useful to avoid having to do a conversion of one of the operands. I wonder if maybe it would be better to add a new method to CStr16 so we could give it a more expressive name than eq, and provide a good place to document the exact behavior. Not sure what a good name would be though... CStr16::eq_str_until_nul maybe? (Got that idea from CStr::from_bytes_until_nul.)

[phip1611]

It differs from the standard library; core::ffi::CStr == str isn't currently allowed

Ahh, I totally agree that uefi-rs should follow conventions and existing APIs from libstd / libcore.

CStr16::eq_str_until_nul

I like the idea, yeah! I update the PR accordingly soon.

[phip1611]

@nicholasbishop I updated the PR. Let me know what you think.

TL;DR: new API:

        let input: &CStr16 = cstr16!("test");

        // test various comparisons with different order (left, right)
        assert!(input.eq_str_until_nul(&"test"));
        assert!(input.eq_str_until_nul(&String::from("test")));

        // now other direction
        assert!(String::from("test").eq_str_until_nul(input));
        assert!("test".eq_str_until_nul(input));

[GabrielMajeri]

Thank you, the code looks good now. I'd ask @nicholasbishop to also take one final look if there's anything else left to improve before merging.

PS: If it's okay for you, I'd be happy to support you as a maintainer/developer in uefi-rs as I'm submitting stuff to uefi-rs anyway from time to time. Just let me know if you need support

I'm not against having more helping hands on board of the project 🙂 What do you say, @nicholasbishop?

[nicholasbishop]

Looks good 👍

PS: If it's okay for you, I'd be happy to support you as a maintainer/developer in uefi-rs as I'm submitting stuff to uefi-rs anyway from time to time. Just let me know if you need support

I'm not against having more helping hands on board of the project 🙂 What do you say, @nicholasbishop?

That sounds good to me as well!