kernel_cmdline: More cleanups and API additions #1490
Conversation
In some cases we want to return the value exactly as it was originally. Also drop the test-only APIs, those were really never needed. Signed-off-by: Colin Walters <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several cleanups and API additions to the kernel_cmdline module. The main change is the introduction of ParameterStr and related functions (find_str, find_all_starting_with_str) to handle UTF-8 kernel arguments in a more type-safe way. The implementation is solid and the new tests cover the added functionality well. My review includes a few suggestions to refine function signatures for better clarity and idiomatic Rust, and to remove a lint suppression.
I don't think anything should use to_lossy() by default. It's great to have a correct kernel argument parser that doesn't bomb on non-UTF8 but at the same time in our code we can just I think ignore kernel arguments which aren't UTF-8. Maybe we should warn if e.g. we find a `root=<nonutf8` or so but eh. Everything else in the bootc codebase works in terms of strings so let's just make it really easy to only get strings out. Implementation notes: - I struggled with lifetimes in this one and couldn't get it to work to reuse the Parameter (byte oriented) parser and just reimplemented it in the str path - When I tossed this problem at both Claude and Gemini they both gave up; and Gemini ended up deleting all the code and declaring success Unit tests (after I manually fixed up all the lifetime stuff in the core code) are Assisted-by: Gemini-CLI Signed-off-by: Colin Walters <[email protected]>
cgwalters
force-pushed
the
kernel-cleanups-3
branch
from
213f879 to
17b4f7b
Compare
|
testing farm looks big stuck here, not sure how to unwedge it and retry... |
Yeah https://status.testing-farm.io/ is up to date, I am hopeful it'll just be another day |
2 participants
No description provided.