Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 0 additions & 36 deletions src/builder.rs

This file was deleted.

14 changes: 14 additions & 0 deletions src/fat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,34 @@ pub fn create_fat_image(
kernel_path: &Path,
) -> io::Result<u32> {
println!("create_fat_image: Starting creation of FAT image.");
println!("create_fat_image: fat_img_path: {:?}", fat_img_path);
println!("create_fat_image: loader_path: {:?}", loader_path);
println!("create_fat_image: kernel_path: {:?}", kernel_path);
Comment on lines +18 to +20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This file includes several println! statements that appear to be for debugging. For better maintainability, it's recommended to use a logging facade like log or tracing. This provides more flexible control over log verbosity and output at runtime compared to hardcoded print statements. These debug prints should be removed before merging.

Comment on lines +18 to +20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These println! statements appear to be for debugging. They should be removed or replaced with a proper logging framework (e.g., the log crate) to provide configurable and structured logging. The same applies to other new println! calls in this function.

Comment on lines +18 to +20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These println! statements appear to be for debugging purposes. For production-quality code, it's better to use a logging facade like the log crate, which allows for configurable log levels and output destinations. These direct prints should be removed to avoid cluttering the standard output of the tool.

Comment on lines +18 to +20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These println! statements appear to be for debugging. It's recommended to remove them or replace them with a proper logging framework (like log or tracing) before merging. This will keep the library's output clean and avoid polluting stdout for consumers of this crate. The same applies to other new println! statements in this function.


// Ensure both files exist
if !loader_path.exists() {
println!(
"create_fat_image: Loader file NOT found at {:?}",
loader_path
);
return Err(io::Error::new(
io::ErrorKind::NotFound,
format!("Loader file not found at {:?}", loader_path),
));
}
println!("create_fat_image: Loader file found at {:?}", loader_path);

if !kernel_path.exists() {
println!(
"create_fat_image: Kernel file NOT found at {:?}",
kernel_path
);
return Err(io::Error::new(
io::ErrorKind::NotFound,
format!("Kernel file not found at {:?}", kernel_path),
));
}
println!("create_fat_image: Kernel file found at {:?}", kernel_path);

// Calculate the minimum image size based on both files
let loader_size = loader_path.metadata()?.len();
Expand Down
50 changes: 31 additions & 19 deletions src/iso/boot_catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,24 @@ pub const BOOT_CATALOG_EFI_PLATFORM_ID: u8 = 0xEF;
pub const ID_FIELD_OFFSET: usize = 4;
pub const BOOT_CATALOG_CHECKSUM_OFFSET: usize = 28;

/// Writes an El Torito boot catalog for UEFI.
pub fn write_boot_catalog(
iso: &mut File,
boot_img_lba: u32,
boot_img_sectors: u16,
) -> io::Result<()> {
pub struct BootCatalogEntry {
pub platform_id: u8,
pub boot_image_lba: u32,
pub boot_image_sectors: u16,
pub bootable: bool,
}

/// Writes an El Torito boot catalog.
pub fn write_boot_catalog(iso: &mut File, entries: Vec<BootCatalogEntry>) -> io::Result<()> {
pad_to_lba(iso, LBA_BOOT_CATALOG)?;

let mut catalog = [0u8; ISO_SECTOR_SIZE];
let mut offset = 0;

// Validation Entry (32 bytes)
let mut val = [0u8; 32];
val[0] = BOOT_CATALOG_VALIDATION_ENTRY_HEADER_ID;
val[1] = BOOT_CATALOG_EFI_PLATFORM_ID;
val[1] = BOOT_CATALOG_EFI_PLATFORM_ID; // This is for the primary boot entry, usually UEFI

// ID string "UEFI" + zero padding (24 bytes total)
val[ID_FIELD_OFFSET..ID_FIELD_OFFSET + 24]
Expand All @@ -46,18 +50,26 @@ pub fn write_boot_catalog(
let checksum = 0u16.wrapping_sub(sum);
val[BOOT_CATALOG_CHECKSUM_OFFSET..BOOT_CATALOG_CHECKSUM_OFFSET + 2]
.copy_from_slice(&checksum.to_le_bytes());
catalog[0..32].copy_from_slice(&val);

// Default Boot Entry (32 bytes)
let mut entry = [0u8; 32];
entry[0] = BOOT_CATALOG_BOOT_ENTRY_HEADER_ID; // Bootable
entry[1] = 0x00; // No Emulation
entry[2..4].copy_from_slice(&0u16.to_le_bytes()); // Load Segment
entry[4] = BOOT_CATALOG_EFI_PLATFORM_ID; // System Type
entry[5] = 0x00; // Unused
entry[6..8].copy_from_slice(&boot_img_sectors.to_le_bytes()); // Sector count (u16, in 512-byte sectors)
entry[8..12].copy_from_slice(&boot_img_lba.to_le_bytes());
catalog[32..64].copy_from_slice(&entry);
catalog[offset..offset + 32].copy_from_slice(&val);
offset += 32;

// Boot Entries
for entry_data in entries {
let mut entry = [0u8; 32];
entry[0] = if entry_data.bootable {
BOOT_CATALOG_BOOT_ENTRY_HEADER_ID
} else {
0x00
}; // Bootable or not
entry[1] = 0x00; // No Emulation
entry[2..4].copy_from_slice(&0u16.to_le_bytes()); // Load Segment
entry[4] = entry_data.platform_id; // System Type
entry[5] = 0x00; // Unused
entry[6..8].copy_from_slice(&entry_data.boot_image_sectors.to_le_bytes()); // Sector count (u16, in 512-byte sectors)
entry[8..12].copy_from_slice(&entry_data.boot_image_lba.to_le_bytes());
catalog[offset..offset + 32].copy_from_slice(&entry);
offset += 32;
}
Comment on lines +57 to +72
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The logic to create boot entries is well-structured. However, if the number of entries becomes large, this loop could potentially write beyond the bounds of the catalog array, which is fixed at ISO_SECTOR_SIZE. The El Torito specification allows for a boot catalog to span multiple sectors, but the current implementation does not support this. For now, it would be safer to add a check to ensure offset does not exceed catalog.len().


iso.write_all(&catalog)
}
Loading