NotificationsYou must be signed in to change notification settings
Fork175
Star1.4k

uefi: Drop the panic-on-logger-errors feature#1143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

phip1611 merged 1 commit intorust-osdev:mainfromnicholasbishop:bishop-drop-panic-log

Apr 21, 2024

Merged

uefi: Drop the panic-on-logger-errors feature#1143

phip1611 merged 1 commit intorust-osdev:mainfromnicholasbishop:bishop-drop-panic-log

Apr 21, 2024

Conversation

Copy link

Member

nicholasbishop commentedApr 20, 2024

This was originally added because of occasional panics when logging quickly on VirtualBox. (Unclear whether this bug is still present, and also as far as I know we haven't observed this behavior on any other UEFI implementations.) The decision was made to panic by default on logger errors, but offer an escape mechanism. However, making this a compile-time choice is not ideal, since most UEFI programs are intended to run on arbitrary UEFI implementations.

We could make it a runtime option instead, but since loggers are usually just informational (i.e. not critical functionality for the application), silently ignoring errors seems like a better choice for most uses.

In the rare case where an application does consider logging critical, they can turn off thelogger helper and implement their own logger.

For prior discussion, see:

Checklist

Sensible git history (for example, squash "typo" or "fix" commits). See theRewriting History guide for help.
Update the changelog (if necessary)

uefi: Drop the panic-on-logger-errors feature

bcac03d

This was originally added because of occasional panics when logging quickly onVirtualBox. (Unclear whether this bug is still present, and also as far as Iknow we haven't observed this behavior on any other UEFI implementations.) Thedecision was made to panic by default on logger errors, but offer an escapemechanism. However, making this a compile-time choice is not ideal, since mostUEFI programs are intended to run on arbitrary UEFI implementations.We could make it a runtime option instead, but since loggers are usually justinformational (i.e. not critical functionality for the application), silentlyignoring errors seems like a better choice for most uses.In the rare case where an application does consider logging critical, they canturn off the `logger` helper and implement their own logger.For prior discussion, see:*rust-osdev#121*rust-osdev#123

phip1611 reviewed

Apr 21, 2024

View reviewed changes

uefi/src/helpers/logger.rs

		// Ignore all errors. Since we're in the logger implementation we
		// can't log the error. We also don't want to panic, since logging
		// is generally not critical functionality.
		let _ = DecoratedLog::write(