From bcac03d6f454d85e097b1901e6bb54a5e6d61c34 Mon Sep 17 00:00:00 2001
From: Nicholas Bishop <nbishop@nbishop.net>
Date: Sat, 20 Apr 2024 12:53:00 -0400
Subject: [PATCH] uefi: Drop the panic-on-logger-errors feature

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 the `logger` helper and implement their own logger.

For prior discussion, see:
* https://github.com/rust-osdev/uefi-rs/issues/121
* https://github.com/rust-osdev/uefi-rs/issues/123
---
 uefi/CHANGELOG.md          |  4 ++++
 uefi/Cargo.toml            |  5 -----
 uefi/src/helpers/logger.rs | 23 +++++------------------
 xtask/src/cargo.rs         |  3 ---
 4 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/uefi/CHANGELOG.md b/uefi/CHANGELOG.md
index dea4a1775..88cbed758 100644
--- a/uefi/CHANGELOG.md
+++ b/uefi/CHANGELOG.md
@@ -1,5 +1,9 @@
 # uefi - [Unreleased]
 
+## Removed
+- Removed the `panic-on-logger-errors` feature of the `uefi` crate. Logger
+  errors are now silently ignored.
+
 
 # uefi - 0.28.0 (2024-04-19)
 
diff --git a/uefi/Cargo.toml b/uefi/Cargo.toml
index d1ba545ba..ce50ed6a2 100644
--- a/uefi/Cargo.toml
+++ b/uefi/Cargo.toml
@@ -13,7 +13,6 @@ repository.workspace = true
 rust-version.workspace = true
 
 [features]
-default = ["panic-on-logger-errors"]
 alloc = []
 
 # Generic gate to code that uses unstable features of Rust. You usually need a nightly toolchain.
@@ -23,10 +22,6 @@ unstable = []
 logger = []
 global_allocator = []
 panic_handler = []
-# Ignore text output errors in logger as a workaround for firmware issues that
-# were observed on the VirtualBox UEFI implementation (see uefi-rs#121).
-# In those cases, this feature can be excluded by removing the default features.
-panic-on-logger-errors = []
 qemu = ["dep:qemu-exit", "panic_handler"] # panic_handler: logical, not technical dependency
 
 [dependencies]
diff --git a/uefi/src/helpers/logger.rs b/uefi/src/helpers/logger.rs
index 3fc386bb1..31e65c3a3 100644
--- a/uefi/src/helpers/logger.rs
+++ b/uefi/src/helpers/logger.rs
@@ -107,30 +107,17 @@ impl log::Log for Logger {
 
         if !output.is_null() {
             let writer = unsafe { &mut *output };
-            let result = DecoratedLog::write(
+
+            // 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(
                 writer,
                 record.level(),
                 record.args(),
                 record.file().unwrap_or("<unknown file>"),
                 record.line().unwrap_or(0),
             );
-
-            // Some UEFI implementations, such as the one used by VirtualBox,
-            // may intermittently drop out some text from SimpleTextOutput and
-            // report an EFI_DEVICE_ERROR. This will be reported here as an
-            // `fmt::Error`, and given how the `log` crate is designed, our main
-            // choices when that happens are to ignore the error or panic.
-            //
-            // Ignoring errors is bad, especially when they represent loss of
-            // precious early-boot system diagnosis data, so we panic by
-            // default. But if you experience this problem and want your UEFI
-            // application to keep running when it happens, you can disable the
-            // `panic-on-logger-errors` cargo feature. If you do so, logging errors
-            // will be ignored by `uefi-rs` instead.
-            //
-            if cfg!(feature = "panic-on-logger-errors") {
-                result.unwrap()
-            }
         }
     }
 
diff --git a/xtask/src/cargo.rs b/xtask/src/cargo.rs
index 78c9e94fb..8376e514e 100644
--- a/xtask/src/cargo.rs
+++ b/xtask/src/cargo.rs
@@ -50,7 +50,6 @@ pub enum Feature {
     Alloc,
     GlobalAllocator,
     Logger,
-    PanicOnLoggerErrors,
     Unstable,
     PanicHandler,
     Qemu,
@@ -70,7 +69,6 @@ impl Feature {
             Self::Alloc => "alloc",
             Self::GlobalAllocator => "global_allocator",
             Self::Logger => "logger",
-            Self::PanicOnLoggerErrors => "panic-on-logger-errors",
             Self::Unstable => "unstable",
             Self::PanicHandler => "panic_handler",
             Self::Qemu => "qemu",
@@ -91,7 +89,6 @@ impl Feature {
                 Self::Alloc,
                 Self::GlobalAllocator,
                 Self::Logger,
-                Self::PanicOnLoggerErrors,
                 Self::Unstable,
                 Self::PanicHandler,
                 Self::Qemu,