knurling-rs / defmt

Efficient, deferred formatting for logging on embedded systems
https://defmt.ferrous-systems.com/
Apache License 2.0
750 stars 69 forks source link

Use UnsafeCell instead of static-mut #817

Open jonathanpallant opened 4 months ago

jonathanpallant commented 4 months ago

The code at https://github.com/knurling-rs/defmt/blob/4db33ca5df8d7a7b5d9036c49b1ea535cc3986c0/firmware/defmt-rtt/src/lib.rs#L140 uses a static mut. It goes to some trouble to ensure that the handle() function cannot be called twice, but perhaps we can use the type system to guarantee that is the case. Or if not, maybe we should use an UnsafeCell.

Also, the buffer is placed in an uninit section but we don't use the MaybeUninit type for the buffer. If there's a reason, we should perhaps explain why.

Urhengulas commented 4 months ago

@japaric Can you shed some light on if there is a reason it should be implemented like it is right now?

evading commented 2 months ago

Also, the buffer is placed in an uninit section but we don't use the MaybeUninit type for the buffer. If there's a reason, we should perhaps explain why.

I was just about to file an issue about that. In the commit that introduced the uninit section in cortex-m-rt it says that MaybeUninit is the only correct way to use that section 1.