Prefer using try_lookup_task for better errors

Author: mkeeter

cmd/dump/src/lib.rs

@@ -427,14 +427,12 @@ fn dump_via_agent(
             if !subargs.simulate_dumper {
                 bail!("--simulate-task-dump requires --simulate-dumper");
             }
-            let ndx = match hubris.lookup_task(task) {
-                Some(HubrisTask::Task(ndx)) => ndx,
-                _ => {
-                    bail!("invalid task \"{task}\"");
-                }
+            let ndx = match hubris.try_lookup_task(task)? {
+                HubrisTask::Task(ndx) => ndx,
+                HubrisTask::Kernel => bail!("invalid task \"{task}\""),
             };
 
-            Some(DumpTask::new(*ndx as u16, hubris.ticks(core)?))
+            Some(DumpTask::new(ndx as u16, hubris.ticks(core)?))
         }
         None => None,
     };
@@ -709,11 +707,9 @@ fn dump_task_via_agent(
     let mut agent = get_dump_agent(hubris, core, subargs)?;
 
     let task = subargs.task.as_ref().unwrap();
-    let ndx = match hubris.lookup_task(task) {
-        Some(HubrisTask::Task(ndx)) => *ndx,
-        _ => {
-            bail!("invalid task \"{task}\"");
-        }
+    let ndx = match hubris.try_lookup_task(task)? {
+        HubrisTask::Task(ndx) => ndx,
+        HubrisTask::Kernel => bail!("invalid task \"{task}\""),
     };
     if ndx == 0 {
         bail!("cannot dump supervisor");

cmd/hiffy/src/lib.rs

@@ -46,7 +46,7 @@
 //!
 
 use ::idol::syntax::{Operation, Reply};
-use anyhow::{Context, Result, anyhow, bail};
+use anyhow::{Context, Result, bail};
 use clap::{CommandFactory, Parser};
 use humility::hubris::*;
 use humility::warn;
@@ -259,11 +259,7 @@ fn hiffy(context: &mut ExecutionContext) -> Result<()> {
         }
 
         let task = match subargs.task {
-            Some(task) => Some(
-                hubris
-                    .lookup_task(&task)
-                    .ok_or_else(|| anyhow!("unknown task \"{}\"", task))?,
-            ),
+            Some(task) => Some(hubris.try_lookup_task(&task)?),
             None => None,
         };
 

cmd/jefe/src/lib.rs

@@ -94,7 +94,7 @@
 //! As with tasks held by `--hold`, use `--release`/`-r` to set the task back to
 //! normal, or `--start`/`-s` to run it once but catch the next fault.
 
-use anyhow::{Result, anyhow, bail};
+use anyhow::{Result, bail};
 use clap::{CommandFactory, Parser};
 use humility::hubris::*;
 use humility_cli::{ExecutionContext, Subcommand};
@@ -150,16 +150,14 @@ fn jefe(context: &mut ExecutionContext) -> Result<()> {
         bail!("one of fault, start, hold, or release must be specified");
     };
 
-    let task = hubris
-        .lookup_task(&subargs.task)
-        .ok_or_else(|| anyhow!("couldn't find task {}", subargs.task))?;
+    let task = hubris.try_lookup_task(&subargs.task)?;
 
     let id = match task {
         HubrisTask::Kernel => {
             bail!("cannot change disposition of kernel");
         }
         HubrisTask::Task(id) => {
-            if let Some(id) = NonZeroU32::new(*id) {
+            if let Some(id) = NonZeroU32::new(id) {
                 id
             } else {
                 bail!("cannot change disposition of supervisor task");

cmd/powershelf/src/lib.rs

@@ -54,7 +54,7 @@ fn lookup_operation_enum(hubris: &HubrisArchive) -> Result<&HubrisEnum> {
         anyhow!("missing `power` task - is this a PSC archive?")
     })?;
 
-    let module = hubris.lookup_module(*power_task)?;
+    let module = hubris.lookup_module(power_task)?;
 
     // `Operation` matches multiple enums, and `task_power_api::Operation`
     // doesn't show up alone, but `Option<task_power_api::Operation>` does! Find

cmd/rpc/src/lib.rs

@@ -389,7 +389,7 @@ impl<'a> RpcClient<'a> {
             )
         })?;
         let rpc_reply_type = hubris
-            .lookup_module(*rpc_task)?
+            .lookup_module(rpc_task)?
             .lookup_enum_byname(hubris, "RpcReply")?
             .ok_or_else(|| anyhow!("can't find RpcReply"))?;
 
@@ -550,11 +550,7 @@ fn rpc_run(context: &mut ExecutionContext) -> Result<()> {
         }
 
         let task = match &subargs.task {
-            Some(task) => Some(
-                hubris
-                    .lookup_task(task)
-                    .ok_or_else(|| anyhow!("unknown task \"{}\"", task))?,
-            ),
+            Some(task) => Some(hubris.try_lookup_task(task)?),
             None => None,
         };
 

humility-core/src/hubris.rs

@@ -1976,15 +1976,15 @@ impl HubrisArchive {
         self.modules.values()
     }
 
-    pub fn lookup_task(&self, name: &str) -> Option<&HubrisTask> {
-        self.tasks.get(name)
+    pub fn lookup_task(&self, name: &str) -> Option<HubrisTask> {
+        self.tasks.get(name).copied()
     }
 
     /// Tries to look up a task by name, returning an error with similar names
     pub fn try_lookup_task(&self, name: &str) -> Result<HubrisTask> {
         self.tasks
             .get(name)
-            .cloned()
+            .copied()
             .ok_or_else(|| self.task_name_suggestion(name))
     }
 
@@ -6684,7 +6684,7 @@ impl ExternRegions {
 
                 for (name, task) in &config.tasks {
                     if task.extern_regions.is_some() {
-                        set.insert(*hubris.lookup_task(name).unwrap());
+                        set.insert(hubris.lookup_task(name).unwrap());
                     }
                 }
 

humility-hiffy/src/lib.rs

@@ -368,7 +368,7 @@ impl<'a> HiffyContext<'a> {
 
                 Some(
                     hubris
-                        .lookup_module(*rpc_task)?
+                        .lookup_module(rpc_task)?
                         .lookup_enum_byname(hubris, "RpcReply")?
                         .ok_or_else(|| anyhow!("failed to find RpcReply"))?,
                 )
@@ -984,7 +984,7 @@ impl<'a> HiffyContext<'a> {
                     None
                 }
 
-                HubrisTask::Task(i) => Some(*i),
+                HubrisTask::Task(i) => Some(i),
             }
         });
 

humility-idol/src/lib.rs

@@ -62,7 +62,7 @@ impl<'a> IdolOperation<'a> {
         hubris: &'a HubrisArchive,
         interface: &str,
         operation: &str,
-        task: Option<&HubrisTask>,
+        task: Option<HubrisTask>,
     ) -> Result<Self> {
         let (task, op, code) = lookup(hubris, interface, operation, task)?;
         let name = (interface.to_string(), operation.to_string());
@@ -309,15 +309,15 @@ fn lookup<'a>(
     hubris: &'a HubrisArchive,
     interface: &str,
     operation: &str,
-    target: Option<&HubrisTask>,
+    target: Option<HubrisTask>,
 ) -> Result<(HubrisTask, &'a Operation, u16)> {
     //
     // Find this interface and its operation.
     //
     let mut rval = None;
 
     let tasks = match target {
-        Some(task) => vec![*task],
+        Some(task) => vec![task],
         None => {
             (0..hubris.ntasks()).map(|t| HubrisTask::Task(t as u32)).collect()
         }