Skip to content

sysfs: return untyped nil for hardware which is not there. - #778

Merged
askervin merged 1 commit into
containers:mainfrom
klihub:fixes/sysfs/return-untyped-nil
Sep 11, 2026
Merged

sysfs: return untyped nil for hardware which is not there.#778
askervin merged 1 commit into
containers:mainfrom
klihub:fixes/sysfs/return-untyped-nil

Conversation

@klihub

@klihub klihub commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

System.CPU, System.Node and System.Package returned a nil pointer inside a non-nil interface for an id the machine does not have. Such a value is never == nil, so callers' nil checks were dead code and the call after them panicked. Return an untyped nil instead, and stop dereferencing without checking in SingleThreadForCPUs and NodeDistance.

@klihub
klihub force-pushed the fixes/sysfs/return-untyped-nil branch from 2c8d91d to 972877f Compare September 10, 2026 10:45
System.CPU, System.Node and System.Package returned a nil
pointer inside a non-nil interface for an id the machine does
not have. Such a value is never == nil, so callers' nil checks
were dead code and the call after them would have panicked.

Return an untyped nil instead, and stop dereferencing without
checking in SingleThreadForCPUs and NodeDistance.

Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@klihub
klihub force-pushed the fixes/sysfs/return-untyped-nil branch from 972877f to 43e9412 Compare September 10, 2026 10:51

@askervin askervin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM.

Yet "already"s in below comments in unit tests were sort of helpful for reviewing this particular PR, they'll be less so after merging this code. So as a super-nit, I'd be fine dropping them.

// Node.DistanceFrom already returns for an unknown destination.
...
// an absent destination was already handled, and still is

@askervin
askervin merged commit 529103a into containers:main Sep 11, 2026
14 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