tosc-rs / mnemos

An Operating System for Building Small Computers
https://mnemos.dev
Apache License 2.0
234 stars 14 forks source link

Question about subscriber implementation #290

Closed Amit-L closed 9 months ago

Amit-L commented 9 months ago

I was just browsing repository this is a minor thing, I came across subscriber implementation here platforms/x86_64/core/src/trace.rs

I am not sure if exit function implementation has intent to enter the span here - https://github.com/tosc-rs/mnemos/blob/416b7d59fbc7fa889a774f54133786a584eb8732/platforms/x86_64/core/src/trace.rs#L90

Thanks

hawkw commented 9 months ago

Nice catch, that's definitely a bug --- it looks like that would result in incorrectly-formatted traces being written to the serial port. We should change that to serial.exit(span).

I think the main reason this wasn't caught earlier is because the x86_64 platform implementation for MnemOS doesn't actually have a working serial driver yet, so we weren't actually writing anything to the serial port in the subscriber, just to the framebuffer. So, nobody noticed wrong traces being written to the serial port because we don't actually do that yet.

If you want to open a PR to change this to call serial.exit(span) instead, I'd be happy to merge it. Otherwise, I'll go ahead and fix this. Thanks for the bug report!

Amit-L commented 9 months ago

I will open PR today, no worries. Thank you