Skip to content

Conversation

@maradini77
Copy link
Contributor

#### Is this resolving a feature or a bug?

Bug fix

#### Are there existing issue(s) that this PR would close?

No

#### Describe your changes.

Replaces panic-prone file reading in `ElfFile::from_path` with proper error handling.

**Problem:**
The method used `std::io::Read::bytes().expect()` which would panic on any I/O error (corrupted file, permission denied, interrupted read), despite returning `Result<Self, VMError>`.

**Solution:**
Changed to use `fs::read()` which properly converts I/O errors to `ParserError::IOError`, maintaining the error propagation contract and preventing unexpected panics.

**Impact:**
- I/O errors are now properly returned instead of causing crashes
- Maintains consistent error handling throughout the ELF loading pipeline
- Improves robustness when dealing with malformed or inaccessible files

@sjudson
Copy link
Contributor

sjudson commented Nov 24, 2025

@maradini77 tests are failing

Copy link
Contributor

@sjudson sjudson left a comment

Choose a reason for hiding this comment

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

One minor nit.


use elf::{endian::LittleEndian, ElfBytes};
use std::fs;
#[cfg(test)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this down to within the existing #[cfg(test)] block around L137?

@maradini77 maradini77 requested a review from sjudson November 24, 2025 17:31
@sjudson sjudson merged commit bdb2a29 into nexus-xyz:main Nov 24, 2025
20 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants