Thoughts about the API

[Strosel]

Hello!
Firstly I'd like to thank you for creating this crate as a rust enthusiast looking forward to developing apps for my flipper zero.

I am starting this discussion thread to voice my own personal opinions and ideas on the API in this crate, and I'm fully aware that my opinion is not universal, but I thought it'd be nice to maybe have some constructive discussion on the topic.

My main point of discussion is the choice of types, mainly in the flipperzero crate and at the time of writing flipperzero::furi::dialog to be specific. What is not gelling with me is the use of CStr throughout this module. I understand that it is necessary "further down" in the sys crate but as I understand it this crate is supposed to be somewhat higher level than the sys crate it is calling into. Therefore wouldn't it make more sense to use an at least somewhat higher-level type and then convert to CStr internally?

I tried cloning the repo and replacing CStr with both str and [u8] as well as changing the affected functions' return types to Result<(), FromBytesWithNulError> to "bubble up" potential conversion errors. I personally thought this worked pretty well, as it made client-side code a little easier to both read and write. The [u8] approach is what I think would work best as it was easier to use inside the crate compared to str with little to no difference client-side.

I look forward to hearing your opinions on this 😄

[dcoles]

Hi @Strosel,

I'm glad to hear you find this crate useful. I hope that it provides a good foundation for Rust apps on the Flipper Zero. 🎉

I 100% agree that working with CStr sucks. There was a little bit of discussion about this very topic in the PR that added dialog support (#24).

The reason we went with CStr is the potential memory overhead of working with &str. The Flipper Zero's SoC is unable to run binaries directly off a microSD card, so any application must be first copied to system memory. This includes string constants defined in .rodata. However, lets say we used &str instead of &CStr in the API. Before we could pass the value to the Flipper Zero's C API we would first need to allocate a new buffer (requiring alloc) to append a ␀ to the end (a second copy of the "string"). Lets say you also wanted to use the format! macro. Now we potentially have the same "string" in memory three times.

Is that too much overhead? It's hard to say. Though I've already been bitten by core::fmt::write! pulling in ~2 MB of formatting code to do nothing more than write!("{}", "Hello, world!")!

So for the moment I'm being conservative, because someone may really need all the memory they can get. This isn't unusual for embedded systems—the Flipper Zero doesn't support exception unwinding for similar reasons.

There is already a flipperzero-sys::c_string! macro, but it would probably make sense to also provide a cstr! and cformat! macro for general use. We were also considering whether to provide a OwnedDialogMessage type that accepted a &str at the cost of it performing an allocation for you.

Let me know what you think. The idea of the flipperzero crate performing CStr::from_bytes_with_nul internally is an interesting one. The main concern would be the cost of doing a ␀ check on every call (and to a lesser extent needing to remember to add \0 everywhere).

[Strosel]

Hello again,

You raise some great points about memory overhead, however, I did some quick investigating using the dialog example and found that the example as-is still contains all string literals in the binary and compiles to ~20KB in release mode. Compare this to using &str or &[u8] with CStr::from_bytes_with_nul inside affected functions which both compile down to ~9KB.

I also looked a bit at the c_string! macro you referenced and found that it returns *const i8 not &CStr wich means we still need to convert to CStr which is the pain point here. This also means we still need to remember the trailing nulls and check for them which as you say might be costly. However, it also appears that all available conversion methods to CStr will perform a null check so it seems impossible to avoid that cost. If nothing else the c_string! macro should return a CStr or &CStr as its name implies.

You also talk about allocation overhead which I can not find myself, this might be due to inexperience but as as far as I can see from the source of str::as_bytes and CStr::from_bytes_with_nul there are no allocations. So it seems any potential allocation overhead is caused only by client-side code.

[dcoles]

The concern is less about the binary size and more about the runtime memory overhead (.rodata + library CString allocation + potential String in user-code).

I think (it's been a while) I used c_string! for *const i8 so cstr! would be available for a &CStr version (or we just use something like the cstr_core crate).

You are correct about str::as_bytes and CStr::from_bytes_with_nul not requiring any allocation (though CStr::from_bytes_with_nul does have to perform a relatively cheap ␀-check). So if the user ensured that all &str/&[u8] had a \0 (rather than the library doing it for you) no allocation is required.

So the options would look something like this:

foo(cstr!("Hello, world!")); // returns void
foo("Hello, world!\0"); // returns `Result<(), FromBytesWithNulError>`

At present I'm still learning towards keeping the type-safe API (&CStr) that lets us do a compile-time rather than run-time check, though I do plan to look how other libraries solve this problem.

[str4d]

As an alternative (and in part because various parts of the Flipper Zero SDK require interacting with this type), I've started wrapping the sys::FuriString type in String/str-like types in #50. While it is an allocating type, it technically can be done without the alloc feature (because it's provided by the SDK itself rather than by the Rust alloc crate), and it is already used in the Flipper Zero firmware in a bunch of places (so we're already paying the cost of the support structure). It wouldn't however work with static strings (as its backing data is whatever sys::FuriString is, rather than raw memory), so &CStr is likely still necessary in various places.