Skip to content

feat: Add format detection logic#62

Merged
mkroening merged 1 commit intohermit-os:mainfrom
fogti:entry-format
Nov 10, 2025
Merged

feat: Add format detection logic#62
mkroening merged 1 commit intohermit-os:mainfrom
fogti:entry-format

Conversation

@fogti
Copy link
Contributor

@fogti fogti commented Nov 6, 2025

This logic is used both by the loader and by uhyve.

This is basically the minimally necessary part of #61.

In general, I would appreciate this PR to be reviewed and merged first. Then I'll port the additional changes from hermit-os/uhyve#1135 into #61 (which have diverged a bit by now).

Copy link
Member

@jounathaen jounathaen left a comment

Choose a reason for hiding this comment

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

I think, this could as well be part of #61, but I think this is good.

Copy link
Member

@mkroening mkroening left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! :)

This is only relevant for the loader and Uhyve, not the kernel, right? Can we gate this with feature = "loader", then? Maybe move this into a loader module?

src/lib.rs Outdated
Comment on lines 40 to 42
if data.len() < 8 {
None
} else if data[0] == 0x7f
Copy link
Member

Choose a reason for hiding this comment

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

I would personally favor early returns over a long if-else chain, but I don't feel strongly about that in this case.

@fogti fogti force-pushed the entry-format branch 3 times, most recently from 0ce282a to fd1aade Compare November 10, 2025 11:25
@fogti fogti requested a review from mkroening November 10, 2025 11:33
This logic is used both by the loader and by uhyve.

Reviewed-by: Martin Kröning <mkroening@posteo.net>
Copy link
Member

@mkroening mkroening left a comment

Choose a reason for hiding this comment

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

Thanks! :)

@mkroening mkroening enabled auto-merge November 10, 2025 12:55
@mkroening mkroening added this pull request to the merge queue Nov 10, 2025
Merged via the queue into hermit-os:main with commit 0492922 Nov 10, 2025
4 checks passed
@fogti fogti deleted the entry-format branch November 10, 2025 13:44
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.

3 participants