search

flipperzero-rs / flipperzero

Rust on the Flipper Zero
MIT License
521 stars 35 forks source link

feat: use now-stable `CStr` literals #120

Closed JarvisCraft closed 8 months ago

JarvisCraft commented 12 months ago

Description

This bumps nightly MSRV to the fresh 2023-12-09 in which CStr-literals have been stabilized and replaces the use of c_string with these literals. The two places where these were previously used for user-defined strings are now replaced with a stricter variant (compile-time validated local CStr creation).

Soundness

Unlike our ad-hoc c_string! macro, the literal ensures that there are no intermediates NUL which is a stricter guarantee.

Since c_string! now does not seem to have a valid use-case, it is removed (which is okay considering that it is a major release).

API design notice

It is worth mentioning that it is still unsound to accept non-validated CStr values as Flipper may have stricter string validity guarantees that just "byte sequence without ZSTs".

From this standpoint, we still may need a new (probably, procedural) macro like furi_str! or utf8_str! with a return-type more strict than CStr (yet still able to be no-alloc).

Thanks to @CertainLach for drawing my attention to this detail.

dcoles commented 10 months ago

It is worth mentioning that it is still unsound to accept non-validated CStr values as Flipper may have stricter string validity guarantees that just "byte sequence without ZSTs".

What part specifically are you thinking might be unsound?

Currently I consider furi/core/string.h and m-string to be the source of truth defining the C FuriString contract (the C FuriString is thin wrapper around a heap-allocated m_string_t).

m-string has the following contract:

  1. A m-string must not be NULL
  2. A m-string's size must be identical to the strlen of the internal character buffer
  3. A m-string must be NUL terminated
  4. A m-string's size may not exceed its current capacity
  5. If a m-string is stack-allocated, then it's capacity must not exceed that of the valid stack region for the string

A couple of consequences of this are:

While an m-string may represent a UTF-8 encoded string, this is not guaranteed (see m_string_utf8_p).

That seems pretty much in-line with the contract of CStr.

If there are specific Flipper Zero APIs that require UTF-8 encoded strings, then we might want to introduce a new-type wrapper for FuriString with this stronger guarantee.

In the context of Rust-based programs, "unsound" specifically means that a "safe" function can be used to trigger behaviour that is considered undefined by the Rust language. Misusing a Flipper Zero API isn't necessarily enough to create unsound behaviour, even if it causes the Flipper Zero to crash, leak memory or lock up.

JarvisCraft commented 8 months ago

@dcoles, I've rebased the PR on master.

It seems that you are right about the relation between m-strings and FuriString, so I have removed the comment from FuriString trait implementation.

What may be the problem though are the graphical APIs which rely on the define of U8G2_WITH_UNICODE for which UTF-8 is actually a requirement:

U8G2_WITH_UNICODE defined

  • C-Code Strings must be UTF-8 encoded
  • Full support of all 65536 glyphs of the unicode basic multilingual plane
  • Up to 65536 glyphs of the font file can be used.

thus I think that we do need a separate type for such string (along with some macro-infra).

I am merging the PR now since there are no significant changes since your approval.

dcoles commented 8 months ago

Hi @JarvisCraft,

Sounds good! Thanks for getting this merged in.