Implement Debug&Display traits for CStr16

[imtsuki]

This is my attempt to implement Debug and Display traits for the CStr16 type using the traits that have already been implemented for Char16.

Strangely if I try to format \u{0}, Display panics while Debug does not, but I don't see any essential differences between the implementations of Char16 for Debug and Display. It seems that the following code is not working for impl Display for Char16:

uefi-rs/src/data_types/chars.rs

Lines 121 to 123 in ba8cb63

	} else {
	write!(f, "{}", core::char::REPLACEMENT_CHARACTER)
	}

This is the panic output:

ERROR: Panic in /Users/tsuki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/src/libcore/result.rs at (1189, 5):
ERROR: called `Result::unwrap()` on an `Err` value: Error

So for now, the formatting process just ends before the null terminator.

Closes #98

Signed-off-by: imtsuki me@qjx.app

[HadrienG2]

Note that it is an explicit invariant of CStr16 that its inner string should not contain a NUL, except for the trailing one which your implementation should not print. So this case should not emerge in practice...

[HadrienG2]

...except if your implementation has an off-by-one, which I suspect it has. The last printable character of a CStr16 is the one at index len-2, but your loop will go up to the trailing NUL at index len-1.

[imtsuki]

In this case, what I'm thinking about is that an Iterator should not interpret its inner data too much and it may be sad for someone trying to manually iterate over a CStr16 and do some low-level operations but the iterator just simply skips the trailing NUL, so I pushed this check into the actual formatting process. Maybe this is not so good, however, because as far as I know std::string in C++11 ends with \0 but its iterator does not return \0. I can't tell which one is better.

[HadrienG2]

I would personally propose not to expose the trailing NUL, for three reasons:

• It is redundant because the Iterator API already has None as a terminator.
• As you observed in this PR, emitting it means that many clients will need to account for its presence and handle it specially, because it is not like the other Char16s in the stream (owing to being in-band metadata rather than actual data).
• From the reverse perspective, I cannot think of any situation beyond simple memcpy where exposing it as part of a read-only string access primitive would eliminate work:
  • Taking substrings still generally requires adding extra NULs.
  • Concatenating strings still requires removing intermediary NULs.
  • ...and memcpy itself can probably be handled via lower-level access than this Iterator, such as the one provided by the to_u16_slice_* methods.

[imtsuki]

I pushed a new commit and now CStr16 skips the trailing NUL.

[HadrienG2]

Also, it's a bit sad that we need to do all this back-and-forth through char, given that CStr16 is what UEFI's text output routines consume in the end. But I don't have a good suggestion of how to avoid this right now.

[imtsuki]

Also, it's a bit sad that we need to do all this back-and-forth through char, given that CStr16 is what UEFI's text output routines consume in the end. But I don't have a good suggestion of how to avoid this right now.

Given that fmt::Write trait only receives &str, I am afraid this kind of cast is unavoidable.

[HadrienG2]

Thanks for this change! I'll merge this as is then.

---

@GabrielMajeri I manually checked the CI log and the failure is not related to this change, but what's in there might be helpful in your ongoing investigation of CI problems. Notice this bit at the end:

INFO: Running UEFI multi-processor services protocol test
**
ERROR:/home/travis/qemu-4.2.0/cpus.c:1795:qemu_tcg_cpu_thread_fn: assertion failed: (cpu->halted)
Subprocess qemu-system-x86_64 exited with error code -6

It seems the test suite does not run through to the end, otherwise you'd see a "shutting down" message. Instead, something bad is going on, most likely in this big MP protocol test.

It would be a good idea to add some verbose logging to this test in order to figure out where exactly it's crashing, and hopefully why.