fix(options): ignore arguments that can't be parsed

An unknown option, an unexpected positional argument, or an invalid
option value made Sprout exit with ABORTED, so a typo in a boot entry or
junk passed by the firmware made the machine unbootable.

Log a warning and drop the argument the parser rejected, then parse the
rest again. Invalid --menu-timeout and --menu-style values are ignored
with a warning too, which also replaces the misleading "invalid integer"
error for an unknown menu style.

crates/boot/src/options.rs

@@ -1,13 +1,12 @@
 use alloc::string::{String, ToString};
-use anyhow::{Context, Result, anyhow};
+use anyhow::{Context, Result, anyhow, bail};
+use core::cell::Cell;
 use core::ptr::null_mut;
 use edera_sprout_config::MenuStyle;
 use jaarg::{
-    ErrorUsageWriter, ErrorUsageWriterContext, HelpWriter, HelpWriterContext, Opt, Opts,
+    HelpWriter, HelpWriterContext, Opt, Opts, ParseControl, ParseResult, StandardFullHelpWriter,
-    ParseControl, ParseError, ParseErrorKind, ParseResult, StandardErrorUsageWriter,
-    StandardFullHelpWriter,
 };
-use log::{error, info};
+use log::{info, warn};
 use uefi_raw::Status;
 
 /// Default configuration file path.
@@ -82,90 +81,105 @@ impl SproutOptions {
         ]);
 
         // Acquire the arguments as determined by the UEFI core.
-        let args = eficore::env::args()?;
+        let mut args = eficore::env::args()?;
 
-        // Use the default value of sprout options and have the raw options be parsed into it.
+        // Parse the arguments, dropping any argument that can't be parsed and parsing again.
-        let mut result = Self::default();
+        // A typo in a boot entry or junk from the firmware should not prevent booting.
+        loop {
+            // Use the default value of sprout options and have the raw options be parsed into it.
+            let mut result = Self::default();
 
-        // Parse the OPTIONS into a map using jaarg.
+            // The number of arguments handed to the parser, used to find the one it rejected.
-        match OPTIONS.parse(
+            let consumed = Cell::new(0usize);
-            "sprout",
+            // The reason the parser rejected an argument, if it did.
-            args.iter(),
+            let mut rejection = None;
-            |program_name, id, _opt, _name, value| {
+
-                match id {
+            // Parse the OPTIONS into a map using jaarg.
-                    ArgID::AutoConfigure => {
+            let parsed = OPTIONS.parse(
-                        // Enable autoconfiguration.
+                "sprout",
-                        result.autoconfigure = true;
+                args.iter().inspect(|_| consumed.set(consumed.get() + 1)),
-                    }
+                |program_name, id, _opt, name, value| {
-                    ArgID::Config => {
+                    match id {
-                        // The configuration file to load.
+                        ArgID::AutoConfigure => {
-                        result.config = value.into();
+                            // Enable autoconfiguration.
-                    }
+                            result.autoconfigure = true;
-                    ArgID::Boot => {
+                        }
-                        // The entry to boot.
+                        ArgID::Config => {
-                        result.boot = Some(value.into());
+                            // The configuration file to load.
-                    }
+                            result.config = value.into();
-                    ArgID::ForceMenu => {
+                        }
-                        // Force showing of the boot menu.
+                        ArgID::Boot => {
-                        result.force_menu = true;
+                            // The entry to boot.
-                    }
+                            result.boot = Some(value.into());
-                    ArgID::MenuTimeout => {
+                        }
-                        // The timeout for the boot menu in seconds.
+                        ArgID::ForceMenu => {
-                        result.menu_timeout = Some(value.parse::<u64>()?);
+                            // Force showing of the boot menu.
-                    }
+                            result.force_menu = true;
-                    ArgID::MenuStyle => {
+                        }
-                        // The style of boot menu to display.
+                        ArgID::MenuTimeout => {
-                        result.menu_style = Some(match value {
+                            // The timeout for the boot menu in seconds.
-                            "basic" => MenuStyle::Basic,
+                            match value.parse::<u64>() {
-                            "simple" => MenuStyle::Simple,
+                                Ok(timeout) => result.menu_timeout = Some(timeout),
-                            // jaarg has no error kind for an unknown choice, but this one is
+                                Err(error) => {
-                            // printed as an invalid argument. The parser fills in the option
+                                    warn!("ignoring invalid {} value '{}': {}", name, value, error)
-                            // and value of the error.
+                                }
-                            _ => {
-                                return Err(ParseError::ArgumentError(
-                                    "",
-                                    "",
-                                    ParseErrorKind::InvalidInteger,
-                                ));
                             }
-                        });
+                        }
+                        ArgID::MenuStyle => {
+                            // The style of boot menu to display.
+                            match value {
+                                "basic" => result.menu_style = Some(MenuStyle::Basic),
+                                "simple" => result.menu_style = Some(MenuStyle::Simple),
+                                _ => warn!(
+                                    "ignoring invalid {} value '{}': expected basic or simple",
+                                    name, value
+                                ),
+                            }
+                        }
+                        ArgID::RetainBootConsole => {
+                            // Retain the boot console before booting.
+                            result.retain_boot_console = true;
+                        }
+                        ArgID::Help => {
+                            let ctx = HelpWriterContext {
+                                options: &OPTIONS,
+                                program_name,
+                            };
+                            info!("{}", StandardFullHelpWriter::new(ctx));
+                            return Ok(ParseControl::Quit);
+                        }
                     }
-                    ArgID::RetainBootConsole => {
+                    Ok(ParseControl::Continue)
-                        // Retain the boot console before booting.
+                },
-                        result.retain_boot_console = true;
+                |_program_name, error| {
-                    }
+                    rejection = Some(error.to_string());
-                    ArgID::Help => {
+                },
-                        let ctx = HelpWriterContext {
+            );
-                            options: &OPTIONS,
-                            program_name,
-                        };
-                        info!("{}", StandardFullHelpWriter::new(ctx));
-                        return Ok(ParseControl::Quit);
-                    }
-                }
-                Ok(ParseControl::Continue)
-            },
-            |program_name, error| {
-                let ctx = ErrorUsageWriterContext {
-                    options: &OPTIONS,
-                    program_name,
-                    error,
-                };
-                error!("{}", StandardErrorUsageWriter::new(ctx));
-            },
-        ) {
-            ParseResult::ContinueSuccess => Ok(result),
-            ParseResult::ExitSuccess => unsafe {
-                uefi::boot::exit(uefi::boot::image_handle(), Status::SUCCESS, 0, null_mut())
-                    .context("unable to exit")?;
-                Err(anyhow!("unable to exit"))
-            },
 
-            ParseResult::ExitError => unsafe {
+            match parsed {
-                uefi::boot::exit(uefi::boot::image_handle(), Status::ABORTED, 0, null_mut())
+                ParseResult::ContinueSuccess => return Ok(result),
-                    .context("unable to exit")?;
+                ParseResult::ExitSuccess => unsafe {
-                Err(anyhow!("unable to exit"))
+                    uefi::boot::exit(uefi::boot::image_handle(), Status::SUCCESS, 0, null_mut())
-            },
+                        .context("unable to exit")?;
+                    return Err(anyhow!("unable to exit"));
+                },
+
+                ParseResult::ExitError => {
+                    // The parser stops at the argument it rejected, which makes it the last
+                    // argument it was handed. This is also true of an option missing its value,
+                    // which is only detected once the arguments run out.
+                    let Some(index) = consumed.get().checked_sub(1).filter(|i| *i < args.len())
+                    else {
+                        bail!("unable to parse options: {}", rejection.unwrap_or_default());
+                    };
+                    let arg = args.remove(index);
+                    warn!(
+                        "ignoring argument '{}': {}",
+                        arg,
+                        rejection.unwrap_or_default()
+                    );
+                }
+            }
         }
     }
 }