spacemeshos / post

Spacemesh POST protocol implementation
MIT License
20 stars 21 forks source link

Fix passing invalid log level to postrs logger #282

Closed poszu closed 7 months ago

poszu commented 7 months ago

The zap noop logger has zapcore.InvalidLevel log level, which is not represented in the go -> C log levels mapping. The current code, for the noop logger, passes a default Level value of 0. This is not valid and causes undefined behavior on the Rust side, which started to cause a crash since Rust 1.76.

The proper behavior is to not set the logger in such case.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 74.4%. Comparing base (8638344) to head (272cb23).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #282 +/- ## ======================================= Coverage 74.3% 74.4% ======================================= Files 29 29 Lines 1545 1549 +4 ======================================= + Hits 1149 1153 +4 Misses 258 258 Partials 138 138 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

fasmat commented 7 months ago

I think we should reconsider not using a global on the Rust side for logging. The logger on the Go side is not a global which especially in tests causes issues. setLogCallback is called multiple times but the oncer inside only once meaning all tests for instance will have the same logging level (as set by the first) in a testrun.

poszu commented 7 months ago

I think we should reconsider not using a global on the Rust side for logging. The logger on the Go side is not a global which especially in tests causes issues. setLogCallback is called multiple times but the oncer inside only once meaning all tests for instance will have the same logging level (as set by the first) in a testrun.

Maybe, but that's not in the scope of this fix.