Skip to content

Prefer using try_lookup_task for better errors#605

Merged
mkeeter merged 1 commit intomasterfrom
mkeeter/try-lookup-task
Mar 11, 2026
Merged

Prefer using try_lookup_task for better errors#605
mkeeter merged 1 commit intomasterfrom
mkeeter/try-lookup-task

Conversation

@mkeeter
Copy link
Contributor

@mkeeter mkeeter commented Mar 10, 2026

Staged on top of #604

That PR added try_lookup_task, which prints similar task names; this PR uses it where appropriate to get better error messages everywhere. It's only appropriate when the task name is provided by the user, not in cases where the name is specified by Humility (e.g. looking up power or udprpc).

It also returns the task by value instead of reference, because they're small Copy-able objects.

@mkeeter mkeeter requested review from hawkw and labbott March 10, 2026 15:40
Comment on lines +430 to +432
let ndx = match hubris.try_lookup_task(task)? {
HubrisTask::Task(ndx) => ndx,
HubrisTask::Kernel => bail!("invalid task \"{task}\""),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right, I think the bail message should just be "cant dump kernel?"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, this was the old message (which applied to both task lookup failures and "task is the kernel"), but we can be more specific now.

Comment on lines +710 to +712
let ndx = match hubris.try_lookup_task(task)? {
HubrisTask::Task(ndx) => ndx,
HubrisTask::Kernel => bail!("invalid task \"{task}\""),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@mkeeter mkeeter force-pushed the mkeeter/per-task-stackmargin branch from 53afeda to 5e9d177 Compare March 11, 2026 18:40
Base automatically changed from mkeeter/per-task-stackmargin to master March 11, 2026 19:40
@mkeeter mkeeter force-pushed the mkeeter/try-lookup-task branch from 01fcadd to 32dac32 Compare March 11, 2026 19:54
@mkeeter mkeeter enabled auto-merge (rebase) March 11, 2026 19:58
@mkeeter mkeeter merged commit 86319f0 into master Mar 11, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants