Feature/hdstorage by Acteus · Pull Request #5 · viralcode/vib-OS

Acteus · 2026-01-18T03:53:06Z

This pull request introduces persistent disk storage support to Vib-OS by adding a block device abstraction, integrating a VirtIO block driver, and enabling FAT32 filesystem mounting from a disk image. It also improves the build system and user experience for disk-backed storage, and enhances file saving in the Notepad app.

Persistent Storage and Filesystem Integration

Added a generic block device interface (block.h) and implemented VirtIO block driver initialization, enabling the kernel to interact with block devices in a unified way. [1] [2]
Modified the kernel initialization to detect and mount a FAT32 filesystem from a VirtIO disk if available, with automatic fallback to RamFS for non-persistent storage. Added logic to initialize the FAT32 driver and mount as root when possible. [1] [2]
Added a shell script (create-disk-image.sh) to create and format a FAT32 disk image, including sample files, for use as persistent storage in QEMU.

QEMU and Build System Enhancements

Updated the Makefile to support new QEMU run modes (run-disk, disk-image), allowing easy testing of persistent storage functionality. Improved help output and added standard C flag. [1] [2] [3] [4]

Userland and Filesystem Usability

Improved file saving in the Notepad GUI app: now uses the O_TRUNC flag to properly overwrite files, and enhanced error handling and logging for save operations.
Updated the VFS open logic to handle O_TRUNC correctly, resetting file size and position when truncating.

These changes together enable persistent user data storage in Vib-OS, improve developer workflow, and enhance the reliability of file operations in the graphical environment.

- Added a new VirtIO block device driver (drivers/block/virtio_blk.c) for persistent storage, enabling read/write operations for FAT32/ext4 filesystems. - Updated the Makefile to include new build targets for running the kernel with persistent disk storage and creating FAT32 disk images. - Introduced a FAT32 filesystem driver (kernel/fs/fat32.c) that supports long filenames and integrates with the VirtIO block driver for disk operations. - Enhanced the kernel initialization process to initialize the VirtIO block driver and attempt to mount a FAT32 filesystem from disk, falling back to RamFS if necessary. - Created a script (scripts/create-disk-image.sh) for generating FAT32 disk images for testing and development purposes. This commit lays the groundwork for persistent storage capabilities in Vib-OS, allowing data to survive reboots.

- Changed CFLAGS in the Makefile to use the GNU11 standard for better compatibility. - Removed unnecessary whitespace in the fat32_file_open function in kernel/fs/fat32.c. - Cleaned up whitespace in the vfs_open function in kernel/fs/vfs.c for improved code readability.

…n main.c

Acteus · 2026-01-18T11:33:39Z

I forgot to add but this is the command to run the os with persistent storage

Create the disk image (first time only)

make disk-image

Run with disk support

make run-disk

viralcode · 2026-01-18T19:52:24Z

@Acteus did you test the filesystem, if so mind pushing some screenshots into this PR ? so i can verify before i merge this. thanks

Acteus · 2026-01-19T14:17:33Z

@Acteus did you test the filesystem, if so mind pushing some screenshots into this PR ? so i can verify before i merge this. thanks

Yeah sure

this is before I create a new folder

then this is after creating and rebooting up the OS

Here is the terminal logs:

Filesystem Testing Evidence

FAT32 Mount - Successful

[INIT] Phase 4: Block Devices & Filesystems
  Initializing VirtIO block driver...
VIRTIO_BLK: Found block device at slot 29
VIRTIO_BLK: Capacity: 131072 sectors (64 MB)
VIRTIO_BLK: Block device initialized successfully
VIRTIO_BLK: Test read successful
  Initializing VFS...
VFS: Initializing virtual filesystem
VFS: Initialized
  Initializing RamFS...
RAMFS: Registering ramfs filesystem
VFS: Registered filesystem 'ramfs'
  Initializing FAT32 driver...
FAT32: Registering FAT32 filesystem
VFS: Registered filesystem 'fat32'
  Attempting to mount FAT32 from disk...
VFS: mount('vda', '/', 'fat32')
FAT32: Mounting filesystem from vda
FAT32: Bytes/sector: 512
FAT32: Sectors/cluster: 1
FAT32: Cluster size: 512 bytes
FAT32: Total clusters: 129022
FAT32: Root cluster: 2
FAT32: FAT size: 1009 sectors (516608 bytes)
FAT32: FAT cached (129152 entries)
FAT32: Filesystem mounted successfully
VFS: Mounted 'vda' on '/'
  Mounted FAT32 filesystem from disk!

File Operations - Working

Create Directory

FAT32: Cluster 6 entry=0x00000000
FAT32: Allocated cluster 6
FAT32: Created directory 'NewFolder' at cluster 6

Create File

FAT32: Allocated cluster 7
FAT32: Created file 'NewFile.txt' at cluster 7
GUI: Created window 'Notepad' (450x350)

Save File Content

Notepad: Save button clicked
FAT32: Updated dir entry size to 0 bytes
FAT32: Truncated file to 0 bytes
FAT32: Updated dir entry size to 68 bytes
Notepad: Saved 68 bytes to /newfolde.r/newfile.txt

viralcode · 2026-01-19T16:21:34Z

the branch has travelled quite a bit after my changes in the kernel, can you merge master in to yours and test everything.. the kernel files seems like conflicting, whatever in master is stuff we need at this point, so keep everything in master and adjust your changes accordingly. appreciate your time.

…AT32 Merged features: - VirtIO block driver for persistent storage - FAT32 filesystem driver with disk-first mount logic - Falls back to RamFS if no disk available - Both run-gpu and run-disk QEMU targets preserved - Stack protector enabled for security (-fstack-protector-strong) - Window resizing, context menus, and enhanced GUI - HD wallpapers and media support - Multi-architecture support (x86, x86_64, arm64) Resolved conflicts in: - Makefile (CFLAGS, run targets) - kernel/core/main.c (Phase 4 filesystem init) - kernel/fs/vfs.c (vfs_open root handling + O_TRUNC) - kernel/gui/window.c (draw_dock, mouse events)

… and improve maintainability.

Copilot

Pull request overview

This pull request adds persistent disk storage support to Vib-OS by implementing a VirtIO block device driver and FAT32 filesystem support. The changes enable the OS to mount and interact with disk-backed storage, with automatic fallback to RAM-based storage when no disk is available.

Changes:

Added VirtIO block driver for disk I/O operations with QEMU's virtio-blk device
Implemented FAT32 filesystem driver with read/write support and directory operations
Enhanced build system with disk image creation script and new QEMU run modes
Improved VFS and file handling with O_TRUNC support

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
scripts/create-disk-image.sh	Shell script to create and format FAT32 disk images for testing
kernel/include/drivers/block.h	Block device abstraction layer API and VirtIO driver declarations
drivers/block/virtio_blk.c	VirtIO block device driver implementation with synchronous I/O
kernel/fs/fat32.c	Complete FAT32 filesystem driver with mount, read, write, and directory operations
kernel/core/main.c	Kernel initialization updated to detect and mount FAT32 or fallback to RamFS
kernel/fs/vfs.c	VFS enhancements for O_TRUNC flag handling and null pointer checks
kernel/gui/window.c	GUI dock drawing enhancements (unrelated to persistent storage feature)
Makefile	New build targets for disk image creation and persistent storage testing

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T13:00:43Z

drivers/block/virtio_blk.c

+static int do_block_io(uint32_t type, uint64_t sector, void *buffer)
+{
+    if (!blk_initialized) {
+        return -1;
+    }
+
+    /* Setup request header */
+    request_hdr.type = type;
+    request_hdr.reserved = 0;
+    request_hdr.sector = sector;
+
+    /* For writes, copy data to our aligned buffer */
+    if (type == VIRTIO_BLK_T_OUT && buffer) {
+        uint8_t *src = (uint8_t *)buffer;
+        for (int i = 0; i < SECTOR_SIZE; i++) {
+            data_buffer[i] = src[i];
+        }
+    }
+
+    /* Initialize status to invalid value */
+    status_byte = 0xFF;
+
+    /* Setup descriptor chain: header -> data -> status */
+    /* Descriptor 0: Request header (device reads) */
+    desc_ring[0].addr = (uint64_t)(uintptr_t)&request_hdr;
+    desc_ring[0].len = sizeof(virtio_blk_req_t);
+    desc_ring[0].flags = VIRTQ_DESC_F_NEXT;
+    desc_ring[0].next = 1;
+
+    /* Descriptor 1: Data buffer */
+    desc_ring[1].addr = (uint64_t)(uintptr_t)data_buffer;
+    desc_ring[1].len = SECTOR_SIZE;
+    if (type == VIRTIO_BLK_T_IN) {
+        /* Read: device writes to buffer */
+        desc_ring[1].flags = VIRTQ_DESC_F_WRITE | VIRTQ_DESC_F_NEXT;
+    } else {
+        /* Write: device reads from buffer */
+        desc_ring[1].flags = VIRTQ_DESC_F_NEXT;
+    }
+    desc_ring[1].next = 2;
+
+    /* Descriptor 2: Status byte (device writes) */
+    desc_ring[2].addr = (uint64_t)(uintptr_t)&status_byte;
+    desc_ring[2].len = 1;
+    desc_ring[2].flags = VIRTQ_DESC_F_WRITE;
+    desc_ring[2].next = 0;
+
+    /* Memory barrier before making descriptor available */
+    mmio_barrier();
+
+    /* Add descriptor chain head to available ring */
+    uint16_t avail_idx = avail_ring.idx;
+    avail_ring.ring[avail_idx % QUEUE_SIZE] = 0;  /* First descriptor index */
+    mmio_barrier();
+    avail_ring.idx = avail_idx + 1;
+    mmio_barrier();
+
+    /* Notify device */
+    mmio_write32(blk_base, VIRTIO_MMIO_QUEUE_NOTIFY, 0);
+
+    /* Wait for completion - poll used ring */
+    int timeout = 10000000;
+    while (used_ring.idx == last_used_idx && timeout > 0) {
+        mmio_barrier();
+        timeout--;
+        /* Small delay */
+        for (volatile int d = 0; d < 10; d++) { }
+    }


The driver uses global static buffers (request_hdr, data_buffer, status_byte defined at lines 144-146) shared across all I/O operations without any locking mechanism. This makes the driver non-reentrant and unsafe for concurrent access. If multiple processes or threads attempt disk I/O simultaneously, these shared buffers will be overwritten, leading to data corruption or incorrect reads/writes. A mutex or spinlock should protect these critical sections, or per-request buffers should be used.

Copilot · 2026-01-20T13:00:44Z

kernel/fs/fat32.c

+    if (zero_cluster(fs, file_cluster) < 0) {
+        /* TODO: Free the allocated cluster on error */
+        return -EIO;
+    }
+
+    /* Add directory entry */
+    uint32_t entry_cluster = 0, entry_offset = 0;
+    if (add_dir_entry(fs, dir_file->start_cluster, dentry->d_name,
+                       FAT_ATTR_ARCHIVE, file_cluster, 0,
+                       &entry_cluster, &entry_offset) < 0) {
+        /* TODO: Free the allocated cluster on error */
+        return -EIO;
+    }


Resource leak on error paths. When zero_cluster fails (line 1005) or add_dir_entry fails (line 1012), the allocated cluster is not freed, permanently consuming disk space. The TODO comments acknowledge this issue but it remains unimplemented. Similarly, if inode allocation fails at line 1044, the fat32_file structure is freed but the disk cluster and directory entry are not cleaned up, causing both memory and disk space leaks.

Copilot · 2026-01-20T13:00:44Z

kernel/fs/fat32.c

+    if (zero_cluster(fs, new_cluster) < 0) {
+        return -EIO;
+    }
+
+    /* Create . and .. entries in the new directory */
+    uint8_t *cluster_buf = kzalloc(fs->cluster_size, GFP_KERNEL);
+    if (!cluster_buf) {
+        return -ENOMEM;
+    }
+
+    struct fat32_dir_entry *entries = (struct fat32_dir_entry *)cluster_buf;
+
+    /* "." entry - points to self */
+    for (int i = 0; i < 8; i++) entries[0].name[i] = ' ';
+    entries[0].name[0] = '.';
+    for (int i = 0; i < 3; i++) entries[0].ext[i] = ' ';
+    entries[0].attr = FAT_ATTR_DIRECTORY;
+    entries[0].cluster_high = (new_cluster >> 16) & 0xFFFF;
+    entries[0].cluster_low = new_cluster & 0xFFFF;
+    entries[0].file_size = 0;
+    entries[0].create_time = 0x6000;
+    entries[0].create_date = 0x5821;
+    entries[0].modify_time = 0x6000;
+    entries[0].modify_date = 0x5821;
+
+    /* ".." entry - points to parent */
+    for (int i = 0; i < 8; i++) entries[1].name[i] = ' ';
+    entries[1].name[0] = '.';
+    entries[1].name[1] = '.';
+    for (int i = 0; i < 3; i++) entries[1].ext[i] = ' ';
+    entries[1].attr = FAT_ATTR_DIRECTORY;
+    uint32_t parent_cluster = dir_file->start_cluster;
+    /* Root directory has cluster 0 for ".." in FAT32 */
+    if (parent_cluster == fs->root_cluster) {
+        parent_cluster = 0;
+    }
+    entries[1].cluster_high = (parent_cluster >> 16) & 0xFFFF;
+    entries[1].cluster_low = parent_cluster & 0xFFFF;
+    entries[1].file_size = 0;
+    entries[1].create_time = 0x6000;
+    entries[1].create_date = 0x5821;
+    entries[1].modify_time = 0x6000;
+    entries[1].modify_date = 0x5821;
+
+    /* Write the new directory cluster */
+    if (write_cluster(fs, new_cluster, cluster_buf) < 0) {
+        kfree(cluster_buf);
+        return -EIO;
+    }
+    kfree(cluster_buf);
+
+    /* Add entry in parent directory */
+    if (add_dir_entry(fs, dir_file->start_cluster, dentry->d_name,
+                       FAT_ATTR_DIRECTORY, new_cluster, 0, NULL, NULL) < 0) {
+        return -EIO;
+    }
+
+    /* Create fat32_file structure for the new directory */
+    struct fat32_file *ff = kzalloc(sizeof(struct fat32_file), GFP_KERNEL);
+    if (!ff) {
+        return -ENOMEM;


Multiple resource leaks on error paths in mkdir. When zero_cluster fails (line 1088), write_cluster fails (line 1133), add_dir_entry fails (line 1140), cluster_buf allocation fails (line 1094), or inode allocation fails (line 1168), the allocated cluster is not freed. Additionally, when write_cluster fails, cluster_buf is properly freed, but subsequent errors don't clean up the written directory cluster. These leaks will permanently consume disk space.

kernel/fs/fat32.c

Copilot · 2026-01-20T13:00:45Z

kernel/core/main.c

  /* Initialize Virtual Filesystem */
  printk(KERN_INFO "  Initializing VFS...\n");


Duplicate print statement detected. Line 163 is a duplicate of line 164, as shown in the diff markers. The "Initialize Virtual Filesystem" print statement appears twice in sequence, which will cause confusing output during boot.

Copilot · 2026-01-20T13:00:45Z

kernel/fs/fat32.c

+
+        if (entry == FAT_ENTRY_FREE) {
+            /* Mark as end of chain */
+            if (set_cluster_entry(fs, cluster, FAT_ENTRY_EOC | 0x0FFFFFF8) == 0) {


Redundant bitwise OR operation. FAT_ENTRY_EOC is already defined as 0x0FFFFFF8, so OR'ing it with 0x0FFFFFF8 is redundant. The expression should just be FAT_ENTRY_EOC or simply use 0x0FFFFFF8 directly. This doesn't cause incorrect behavior but makes the code confusing.

Suggested change

if (set_cluster_entry(fs, cluster, FAT_ENTRY_EOC | 0x0FFFFFF8) == 0) {

if (set_cluster_entry(fs, cluster, FAT_ENTRY_EOC) == 0) {

Copilot · 2026-01-20T13:00:45Z

scripts/create-disk-image.sh

+    TMPDIR=$(mktemp -d)
+    echo "Welcome to Vib-OS!" > "$TMPDIR/readme.txt"
+    echo "This file is stored on a persistent FAT32 disk." >> "$TMPDIR/readme.txt"
+    echo "Data will survive reboots!" >> "$TMPDIR/readme.txt"
+
+    # Copy to disk image
+    mcopy -i "$OUTPUT_PATH" "$TMPDIR/readme.txt" ::/README.TXT 2>/dev/null || true
+    mmd -i "$OUTPUT_PATH" ::/DOCUMENTS 2>/dev/null || true
+
+    rm -rf "$TMPDIR"


Using TMPDIR as a variable name conflicts with the standard TMPDIR environment variable that controls where mktemp creates temporary directories. If TMPDIR is already set in the environment, this assignment might not work as expected, or it could cause mktemp to use an unexpected location. Consider using a different variable name like TMP_DIR or SAMPLE_DIR to avoid this conflict.

Suggested change

TMPDIR=$(mktemp -d)

echo "Welcome to Vib-OS!" > "$TMPDIR/readme.txt"

echo "This file is stored on a persistent FAT32 disk." >> "$TMPDIR/readme.txt"

echo "Data will survive reboots!" >> "$TMPDIR/readme.txt"

# Copy to disk image

mcopy -i "$OUTPUT_PATH" "$TMPDIR/readme.txt" ::/README.TXT 2>/dev/null || true

mmd -i "$OUTPUT_PATH" ::/DOCUMENTS 2>/dev/null || true

rm -rf "$TMPDIR"

TMP_DIR=$(mktemp -d)

echo "Welcome to Vib-OS!" > "$TMP_DIR/readme.txt"

echo "This file is stored on a persistent FAT32 disk." >> "$TMP_DIR/readme.txt"

echo "Data will survive reboots!" >> "$TMP_DIR/readme.txt"

# Copy to disk image

mcopy -i "$OUTPUT_PATH" "$TMP_DIR/readme.txt" ::/README.TXT 2>/dev/null || true

mmd -i "$OUTPUT_PATH" ::/DOCUMENTS 2>/dev/null || true

rm -rf "$TMP_DIR"

Copilot · 2026-01-20T13:00:45Z

kernel/fs/fat32.c

+    /* Set timestamps (use a fixed date for simplicity: 2024-01-01 12:00:00) */
+    /* Time: hours=12, minutes=0, seconds=0 -> (12<<11)|(0<<5)|0 = 0x6000 */
+    /* Date: year=2024-1980=44, month=1, day=1 -> (44<<9)|(1<<5)|1 = 0x5821 */
+    entry->create_time = 0x6000;
+    entry->create_date = 0x5821;
+    entry->modify_time = 0x6000;
+    entry->modify_date = 0x5821;
+    entry->access_date = 0x5821;


The hardcoded timestamp uses a date in the past (2024-01-01), which is more than a year old given the current date of January 20, 2026. This will cause newly created files and directories to have incorrect creation and modification dates. Consider using the current system time or updating the hardcoded value to be closer to the current date.

Copilot · 2026-01-20T13:00:46Z

kernel/fs/vfs.c

+  /* Handle O_TRUNC - reset file position and size to 0 */
+  if (flags & O_TRUNC) {
+    f->f_pos = 0;
+    child->d_inode->i_size = 0;
+    /* Truncate operation will be handled by filesystem on write */
+  }
+


O_TRUNC is handled twice: once by the filesystem's open callback (line 345 calls f->f_op->open which for FAT32 handles O_TRUNC at lines 812-824 of fat32.c), and then again at lines 348-353 in VFS. This double handling could cause race conditions or inconsistencies. The VFS-level handling should be removed since the filesystem driver's open callback already handles O_TRUNC properly, or the filesystem callback should not handle it and leave it to VFS.

Suggested change

/* Handle O_TRUNC - reset file position and size to 0 */

if (flags & O_TRUNC) {

f->f_pos = 0;

child->d_inode->i_size = 0;

/* Truncate operation will be handled by filesystem on write */

}

Copilot · 2026-01-20T13:00:46Z

kernel/fs/fat32.c

+    /* TODO: Allocate new clusters if needed */
+    /* For now, only support writing within existing file bounds */


The write operation does not allocate new clusters when writing beyond existing file bounds. The TODO comment at line 765 confirms this limitation. This means that growing files or writing to truncated files will fail silently after consuming all initially allocated clusters, which severely limits the functionality of persistent storage. The write will stop early without reporting an error to the caller, potentially causing data loss.

…minal scrolling and wallpaper caching implementation. Adjust mouse event handling and button layout for notepad functionality. Clean up dock drawing logic and prepare for future tooltip implementation.

Acteus · 2026-01-20T13:07:44Z

@viralcode I resolved the conflicts, should be good to merge now. I don't think I've missed anything, but hmu if I broke or missed something

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 15 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T13:15:46Z

drivers/block/virtio_blk.c

+    mmio_write32(blk_base, VIRTIO_MMIO_QUEUE_NOTIFY, 0);
+
+    /* Wait for completion - poll used ring */
+    int timeout = 10000000;


The magic number 10000000 is used as a timeout value without explanation. This makes it difficult to understand the actual timeout duration or adjust it if needed. Consider defining this as a named constant with a comment explaining the expected timeout duration.

Copilot · 2026-01-20T13:15:47Z

kernel/gui/window.c

 static char term_history[TERM_HISTORY_LINES][80];
 static int term_history_count = 0;
-static int term_scroll = 0;
+/* static int term_scroll = 0; */ /* TODO: implement terminal scrolling */


The comment indicates that terminal scrolling is not implemented. If this is planned functionality, consider tracking it in an issue tracker rather than leaving a TODO comment in the code. If it's not needed, remove the commented-out variable entirely.

Suggested change

/* static int term_scroll = 0; */ /* TODO: implement terminal scrolling */

Copilot · 2026-01-20T13:15:47Z

kernel/gui/window.c

+  /* TODO: Use these for button tooltips */
+  /* const char *icons[] = {"<", ">", "R", "L", "+", "-", "F", "X"}; */
+  /* const char *labels[] = {"Prev", "Next", "Rot R", "Rot L", "Zoom+", "Zoom-", "Fit", "Full"}; */


The comment "TODO: Use these for button tooltips" suggests planned functionality for tooltips using the icons and labels arrays, but the arrays are commented out. Either implement the tooltip functionality or remove the commented-out code. Leaving it commented creates maintenance burden and code clutter.

Copilot · 2026-01-20T13:15:47Z

kernel/fs/fat32.c

+    .kill_sb = fat32_kill_sb,
+    .next = NULL,
+};
+
+/* ===================================================================== */
+/* Initialization */
+/* ===================================================================== */
+
+int fat32_init(void)
+{
+    printk(KERN_INFO "FAT32: Registering FAT32 filesystem\n");
+    return register_filesystem(&fat32_fs_type);
+}
+
+/* ===================================================================== */
+/* Public API */
+/* ===================================================================== */


The error handling in the mount function has duplicate cleanup code for freeing fs and its components. The cleanup at lines 1404-1410 repeats the same logic as lines 1369-1375. This duplication makes the code harder to maintain and increases the risk of bugs. Consider consolidating the cleanup logic into a single error handling path or using goto-based cleanup.

Copilot · 2026-01-20T13:15:48Z

kernel/fs/fat32.c

+    for (size_t i = 0; i < sizeof(struct fat32_dir_entry); i++) {
+        ((uint8_t *)entry)[i] = 0;
+    }


The manual memory clearing loop is inefficient. Consider using memset for better performance and cleaner code.

Copilot · 2026-01-20T13:15:49Z

kernel/fs/fat32.c

+    for (size_t i = 0; i < sizeof(struct fat32_bpb); i++) {
+        ((uint8_t *)&fs->bpb)[i] = boot_sector[i];
+    }


The manual byte-by-byte memory copy is inefficient. Consider using memcpy for better performance and cleaner code.

Copilot · 2026-01-20T13:15:50Z

kernel/include/drivers/block.h

+int block_register(struct block_device *dev);
+
+/**
+ * block_unregister - Unregister a block device
+ * @dev: Block device to unregister
+ */
+void block_unregister(struct block_device *dev);
+
+/**
+ * block_get_device - Get a block device by name
+ * @name: Device name
+ * Returns: Pointer to device, or NULL if not found
+ */
+struct block_device *block_get_device(const char *name);
+
+/**
+ * block_get_device_by_index - Get a block device by index
+ * @index: Device index (0-based)
+ * Returns: Pointer to device, or NULL if invalid index
+ */
+struct block_device *block_get_device_by_index(int index);
+
+/* ===================================================================== */
+/* Block I/O functions */
+/* ===================================================================== */
+
+/**
+ * block_read - Read sectors from a block device
+ * @dev: Block device
+ * @sector: Starting sector number
+ * @count: Number of sectors to read
+ * @buffer: Destination buffer (must be count * sector_size bytes)
+ * Returns: 0 on success, negative error code on failure
+ */
+int block_read(struct block_device *dev, uint64_t sector,
+               uint32_t count, void *buffer);
+
+/**
+ * block_write - Write sectors to a block device
+ * @dev: Block device
+ * @sector: Starting sector number
+ * @count: Number of sectors to write
+ * @buffer: Source buffer (must be count * sector_size bytes)
+ * Returns: 0 on success, negative error code on failure
+ */
+int block_write(struct block_device *dev, uint64_t sector,
+                uint32_t count, const void *buffer);
+
+/**
+ * block_flush - Flush cached data to device
+ * @dev: Block device
+ * Returns: 0 on success, negative error code on failure
+ */
+int block_flush(struct block_device *dev);


The block device abstraction defines a generic interface but none of the functions (block_register, block_unregister, block_get_device, block_get_device_by_index, block_read, block_write, block_flush) appear to be implemented anywhere in this PR. The header promises a unified API for filesystem drivers, but the FAT32 driver directly calls virtio_blk_* functions instead of using the block device abstraction. This suggests incomplete implementation or dead code in the header.

Copilot · 2026-01-20T13:15:50Z

kernel/core/main.c

+  fat32_init();
+
+  /* Try to mount FAT32 from disk first, fall back to RamFS */
+  int mounted_disk = 0;
+  if (blk_ret == 0 && virtio_blk_is_ready()) {


The initialization sequence attempts to mount FAT32 first, then falls back to RamFS. However, if FAT32 initialization (fat32_init) fails silently, the code still attempts to mount it. Consider checking the return value of fat32_init and only attempting FAT32 mount if initialization succeeded.

Suggested change

fat32_init();

/* Try to mount FAT32 from disk first, fall back to RamFS */

int mounted_disk = 0;

if (blk_ret == 0 && virtio_blk_is_ready()) {

int fat_ret = fat32_init();

if (fat_ret != 0) {

printk(KERN_WARNING " FAT32 driver initialization failed (ret=%d), will not attempt FAT32 mount\n",

fat_ret);

}

/* Try to mount FAT32 from disk first, fall back to RamFS */

int mounted_disk = 0;

if (blk_ret == 0 && fat_ret == 0 && virtio_blk_is_ready()) {

Copilot · 2026-01-20T13:15:50Z

kernel/fs/fat32.c

+    root_dentry.d_parent = &root_dentry;
+    root_dentry.d_child = NULL;
+    root_dentry.d_sibling = NULL;
+    root_dentry.d_sb = &sb;
+
+    sb.s_root = &root_dentry;
+    mounted_fs = fs;
+
+    printk(KERN_INFO "FAT32: Filesystem mounted successfully\n");
+
+    return &sb;
+}
+
+static void fat32_kill_sb(struct super_block *sb)
+{
+    struct fat32_fs *fs = (struct fat32_fs *)sb->s_fs_info;


The memory leak cleanup logic in the error path does not free the root_file structure that was allocated at line 1331. This will leak memory if the mount operation fails after allocating root_file but before completing successfully.

Copilot · 2026-01-20T13:15:50Z

kernel/fs/fat32.c

+        /* TODO: Free the allocated cluster on error */
+        return -EIO;
+    }
+
+    /* Add directory entry */
+    uint32_t entry_cluster = 0, entry_offset = 0;
+    if (add_dir_entry(fs, dir_file->start_cluster, dentry->d_name,
+                       FAT_ATTR_ARCHIVE, file_cluster, 0,
+                       &entry_cluster, &entry_offset) < 0) {
+        /* TODO: Free the allocated cluster on error */
+        return -EIO;


The TODO comments indicate that error recovery for allocated clusters is missing. When file or directory creation fails after allocating a cluster, the cluster is not freed, leading to disk space leakage. This should be implemented to prevent filesystem corruption and wasted space over time.

Acteus added 3 commits January 18, 2026 11:43

Remove unused global terminal pointer and clean up keyboard handler i…

0bbee0a

…n main.c

Copilot AI review requested due to automatic review settings January 20, 2026 12:54

Copilot started reviewing on behalf of Acteus January 20, 2026 12:54 View session

Remove unused draw_dock function from window.c to streamline GUI code…

05e0757

… and improve maintainability.

Copilot AI reviewed Jan 20, 2026

View reviewed changes

Refactor window.c: Comment out unused variables and add TODOs for ter…

da1810d

…minal scrolling and wallpaper caching implementation. Adjust mouse event handling and button layout for notepad functionality. Clean up dock drawing logic and prepare for future tooltip implementation.

Copilot AI review requested due to automatic review settings January 20, 2026 13:08

Copilot started reviewing on behalf of Acteus January 20, 2026 13:08 View session

Acteus force-pushed the feature/hdstorage branch from 2111ba0 to da1810d Compare January 20, 2026 13:14

Copilot AI reviewed Jan 20, 2026

View reviewed changes

		/* Initialize Virtual Filesystem */
		printk(KERN_INFO " Initializing VFS...\n");

	if (set_cluster_entry(fs, cluster, FAT_ENTRY_EOC \| 0x0FFFFFF8) == 0) {
	if (set_cluster_entry(fs, cluster, FAT_ENTRY_EOC) == 0) {

		/* TODO: Allocate new clusters if needed */
		/* For now, only support writing within existing file bounds */

Conversation

Acteus commented Jan 18, 2026

Uh oh!

Acteus commented Jan 18, 2026

Create the disk image (first time only)

Run with disk support

Uh oh!

viralcode commented Jan 18, 2026

Uh oh!

Acteus commented Jan 19, 2026

Filesystem Testing Evidence

FAT32 Mount - Successful

File Operations - Working

Create Directory

Create File

Save File Content

Uh oh!

viralcode commented Jan 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Acteus commented Jan 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026