kernel_cmdline: Simplify Display impl for utf8::Parameter

[jeckersb]

Really we want to just return the original slice that the Parameter
was created from. This also aligns with how Cmdline and ParameterKey
impl Display.

Where this really matters the most is to ensure we retain the quoting
that the parameter was created with, so I added a test just to sanity
check that. Before this change the test would fail because "foo"
would be stripped of its quotes and just rendered as foo unquoted.

Signed-off-by: John Eckersberg jeckersb@redhat.com

[gemini-code-assist]

Code Review

This pull request simplifies the Display implementation for utf8::Parameter to correctly return the original string slice, which is a great improvement for both correctness and clarity. The change ensures that quoting is preserved, and the new test case effectively validates this behavior. I have one minor suggestion to make the implementation even more idiomatic.

[bshephar]

from_utf8 is a safe operation right? Isn't it from_utf8_unchecked that is the unsafe method?
Safe:
https://doc.rust-lang.org/std/primitive.str.html#method.from_utf8

If you are sure that the byte slice is valid UTF-8, and you don’t want to incur the overhead of the validity check, there is an unsafe version of this function, from_utf8_unchecked, which has the same behavior but skips the check.

Unsafe:
https://doc.rust-lang.org/std/str/fn.from_utf8_unchecked.html

Just wondering if the safety comment is necessary here. Seems like it would only be necessary if using the unchecked version.

[jeckersb]

Yeah it's safe and probably not correct to the letter of the law of rust conventions to label it like that. It's like that because in the very earliest version of this code, it did actually use from_utf8_unchecked. But really it's not so performance critical that we need to squeeze out a few cycles and avoid the safe code. So we switched it to do the checked version and take the performance "penalty". In theory however, those comments still hold true and we should never hit the case where the underlying bytes are invalid UTF-8 and if we really really wanted to we could switch back to using the unchecked version.

Or put another way... it should be 100% impossible for this to panic. But if something does goes wrong, at least it will panic instead of undefined behavior.

[henrywang]

Just make a rebase for test install failure.

[jeckersb]

Hm not sure what the testing farm failures on stream 10 are about:

        Command 'reboot' returned 1.

        # stderr (1 lines)
        # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        Call to Reboot failed: Access denied
        # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Unrelated to this change but worth noting.