FAT32 - Can't read file names

[deleted8917]

Again me, now I'm trying to implement FAT32 filesystem support.
I successfully abled to read the BPB and MBR, but now when I try to read the names of the files that are in the root directory, it doesn't work. Just displays "no more files"
The virtual harddisk image is formatted in FAT32 using fdisk.

fat32.c

Code: Select all

#include <stdint.h>
#include <drivers/storage/hdd/hd.h>
#include <kernel/terminal.h>
#include <stdbool.h>

#define FATBPB_SIZE sizeof(struct FATBPB)
#define FAT32_FILES_PER_DIRECTORY 16

struct FATBPB
{
    uint8_t jump[3];
    uint8_t oem_id[8];
    uint16_t bytes_per_sector;
    uint8_t sectors_per_cluster;
    uint16_t reserved_sectors;
    uint8_t fats_number;
    uint16_t dir_entries;
    uint16_t logical_sectors;
    uint8_t media_type;
    uint16_t sectors_fat; /* FAT12- FAT16 only! */
    uint16_t sectors_per_track;
    uint16_t head_number;
    uint32_t hidden_sectors;
    uint32_t large_sector_count;
    
    /* EBPB */
    uint32_t sectors_per_fat;
    uint16_t flags;
    uint16_t fat_ver;
    uint32_t cluster_root;
    uint16_t fat_info;
    uint32_t backup_sector;
    uint8_t res[12];
    uint8_t drive_number;
    uint8_t res2;
    uint8_t signature;
    uint32_t volume_id;
    uint8_t volume_label[11];
    uint8_t boot_code[8];
    uint16_t file_system;
}__attribute__((packed));

struct DirectoryEntry
{
    uint8_t file_name[11];
    uint8_t attributes;
    uint8_t res;
    uint8_t ctimeT;
    uint16_t ctime;
    uint16_t cdate;
    uint16_t last_access;
    uint16_t cluster_number_hi;
    uint16_t lmtime;
    uint16_t lmdate;
    uint16_t cluster_number_lo;
    uint32_t file_size;
}__attribute__((packed));

struct FATInfo
{
    bool fixed_media;
};

/* Global declaration of an FAT BPB struct */
struct FATBPB bpb;

void read_bpb(uint32_t offset)
{
    hd_read(offset, FATBPB_SIZE, (uint8_t*)&bpb);
    
    struct DirectoryEntry drce[FAT32_FILES_PER_DIRECTORY];

    uint32_t fat_start = (offset + bpb.reserved_sectors);
    uint32_t fat_size = bpb.sectors_per_fat;
    uint32_t start_of_data = fat_start * bpb.fats_number;
    uint32_t start_of_root = (start_of_data + bpb.sectors_per_cluster) * (bpb.cluster_root - 2);
    
    hd_read(start_of_root, FAT32_FILES_PER_DIRECTORY * sizeof(struct DirectoryEntry), (uint8_t*)&drce[0]);
    for (int i = 0; i < FAT32_FILES_PER_DIRECTORY; ++i) {
        if (drce[i].file_name[0] == 0x00) {
            kputs("\n-- No more files in the directory --\n");
            break;
        }
        if ((drce[i].attributes & 0x0F) == 0x0F)
            continue;

        /* Horrible solution, I'll fix this later, I promise  */
        char* asd = "        \n";
        for (int j = 0; j < 8; ++j) {
            asd[j] = drce[i].file_name[j];
        }
        /* This should print the names of the files that are in the root directory... */
        kputs(asd);
        if ((drce[i].attributes & 0x10) == 0x10)
            continue;
        uint32_t fcluster = ((uint32_t)drce[i].cluster_number_hi) << 16 | ((uint32_t)drce[i].cluster_number_lo);
        uint32_t fsect = start_of_data + bpb.sectors_per_cluster * (fcluster - 2);
        uint8_t buff[512];
        hd_read(fsect, 512, buff);
        buff[drce[i].file_size] = '\0';
        kputs("%s", (char*)buff);
    }
}

/* Returs FALSE if the media is fixed, true if otherwise */
bool fat32_media_type(void)
{
    /* All these values are valid for removable media, e.g: USB, floppy disk, etc */
    uint8_t removable_media_valid_values[9] = {0xF0, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF};
    for (int i = 0; i < 9; ++i)
        if (bpb.media_type == removable_media_valid_values[i])
            return true;

    /* Media is an fixed one, e.g: HDD */
    if (bpb.media_type == 0xF8) {
        kputs("Fixed media\n");
        return false;
    }
}

/* Returns the volume label */
char* fat32_get_label(void)
{
    return bpb.volume_label;
}

As ever, thanks.

[quirck]

Check the structures: hidden_sectors and large_sector_count should be uint32_t, and in DirectoryEntry file_name should have 11 characters. Also, check the math of finding the root cluster: I doubt that fat_start should be multiplied by fats_number, etc.

[deleted8917]

Tried that, getting the same behaviour.

[Octocontrabass]

Tried that, getting the same behaviour.

What did you try? The code in your first post still has wrong structs, and still has wrong calculations for the location of the first sector in the root directory.

[MichaelPetch]

Have you started using a debugger for this kind of thing? I can't stress enough the importance of learning to use a debugger. Since you are in protected mode at this point QEMU would be good choice (GDB can make a remote connection to QEMU). You can do symbolic debugging; set a breakpoint; dump out the entire contents of data structures (like the BPB structure) when read into memory. You would have realized that data inside the BPB looks incorrect. Some of your data structures have bugs (wrong sizes). You also have bugs in your calculations where parentheses are in the wrong place. A slightly revised version (I have commented where I've made changes):

Code: Select all

#include <stdint.h>
#include <drivers/storage/hdd/hd.h>
#include <kernel/terminal.h>
#include <stdbool.h>

#define FATBPB_SIZE sizeof(struct FATBPB)
#define FAT32_FILES_PER_DIRECTORY 16

struct FATBPB
{
    uint8_t jump[3];
    uint8_t oem_id[8];
    uint16_t bytes_per_sector;
    uint8_t sectors_per_cluster;
    uint16_t reserved_sectors;
    uint8_t fats_number;
    uint16_t root_dir_entries;
    uint16_t logical_sectors;
    uint8_t media_type;
    uint16_t sectors_fat; /* FAT12- FAT16 only! */
    uint16_t sectors_per_track;
    uint16_t head_number;
    uint32_t hidden_sectors;             /* Fixed */
    uint32_t large_sector_count;         /* Fixed */

    uint32_t sectors_per_fat;
    uint16_t flags;
    uint16_t fat_ver;
    uint32_t cluster_root;
    uint16_t fat_info;
    uint16_t backup_sector;              /* Fixed */
    uint8_t res[12];
    uint8_t drive_number;
    uint8_t res2;
    uint8_t signature;
    uint32_t volume_id;
    uint8_t label[11];
    uint8_t sysid[8];                    /* Fixed - rename */
                                         /* Remove boot code and sig as not part of BPB */
}__attribute__((packed));

struct DirectoryEntry
{
    uint8_t file_name[11];               /* Fixed 8 -> 11 */
    uint8_t attributes;
    uint8_t res;
    uint8_t ctimeT;
    uint16_t ctime;
    uint16_t cdate;
    uint16_t last_access;
    uint16_t cluster_number_hi;
    uint16_t lmtime;
    uint16_t lmdate;
    uint16_t cluster_number_lo;
    uint32_t file_size;
}__attribute__((packed));

void read_bpb(uint32_t offset)
{
    struct FATBPB bpb;
    hd_read(offset, FATBPB_SIZE, (uint8_t*)&bpb);
    struct DirectoryEntry drce[FAT32_FILES_PER_DIRECTORY];

    uint32_t fat_start = (offset + bpb.reserved_sectors);
    uint32_t fat_size = bpb.sectors_per_fat;
    /* Fixed these calculations */
    uint32_t start_of_data = fat_start + (fat_size * bpb.fats_number);
    uint32_t start_of_root = start_of_data + ((bpb.cluster_root - 2) * bpb.sectors_per_cluster);

    hd_read(start_of_root,
            FAT32_FILES_PER_DIRECTORY * sizeof(struct DirectoryEntry),
            (uint8_t*)&drce[0]);
    for (int i = 0; i < FAT32_FILES_PER_DIRECTORY; ++i) {
        if (drce[i].file_name[0] == 0x00) {
            kputs("\n-- No more files in the directory --\n");
            break;
        }
        if ((drce[i].attributes & 0x0F) == 0x0F)
            continue;

        /* Horrible solution, I'll fix this later, I promise
         * Fixed: put on stack to avoid writing to readonly data */
        char asd[] = "        \n";
        for (int j = 0; j < 8; ++j) {
            asd[j] = drce[i].file_name[j];
        }
        /* This should print the names of the files that are in the root directory... */
        kputs(asd);
        if ((drce[i].attributes & 0x10) == 0x10)
            continue;
        
        uint32_t fcluster = ((uint32_t)drce[i].cluster_number_hi) << 16 | ((uint32_t)drce[i].cluster_number_lo);
        uint32_t fsect = start_of_data + bpb.sectors_per_cluster * (fcluster - 2);
        uint8_t buff[512];
        hd_read(fsect, 512, buff);
        buff[drce[i].file_size] = '\0';
        kputs("%s", (char*)buff);
    }
}

As well your asd buffer was defined in such a way that you invoke undefined behavior when writing to it. Writing to read only data isn't a good idea. I modified your code to define asd's to be allocated on the stack via []. Since we don't see hd_read we can't tell if that code is buggy and causing other issues.

[MichaelPetch]

I made some adjustments to my original answer and removed some of the code related to the start_of_root so it closer to your original. That part is now very similar to yours except I have moved the brackets on the equation around so they generate the correct result.

[deleted8917]

Tried that, getting the same behaviour.

What did you try? The code in your first post still has wrong structs, and still has wrong calculations for the location of the first sector in the root directory.

No, I actually tried, just I forgetted to update the code that is in the thread, I always forget that... Updated now, sorry for that.

[deleted8917]

Have you started using a debugger for this kind of thing? I can't stress enough the importance of learning to use a debugger. Since you are in protected mode at this point QEMU would be good choice (GDB can make a remote connection to QEMU). You can do symbolic debugging; set a breakpoint; dump out the entire contents of data structures (like the BPB structure) when read into memory. You would have realized that data inside the BPB looks incorrect. Some of your data structures have bugs (wrong sizes). You also have bugs in your calculations where parentheses are in the wrong place. A slightly revised version (I have commented where I've made changes):

Code: Select all

#include <stdint.h>
#include <drivers/storage/hdd/hd.h>
#include <kernel/terminal.h>
#include <stdbool.h>

#define FATBPB_SIZE sizeof(struct FATBPB)
#define FAT32_FILES_PER_DIRECTORY 16

struct FATBPB
{
    uint8_t jump[3];
    uint8_t oem_id[8];
    uint16_t bytes_per_sector;
    uint8_t sectors_per_cluster;
    uint16_t reserved_sectors;
    uint8_t fats_number;
    uint16_t root_dir_entries;
    uint16_t logical_sectors;
    uint8_t media_type;
    uint16_t sectors_fat; /* FAT12- FAT16 only! */
    uint16_t sectors_per_track;
    uint16_t head_number;
    uint32_t hidden_sectors;             /* Fixed */
    uint32_t large_sector_count;         /* Fixed */

    uint32_t sectors_per_fat;
    uint16_t flags;
    uint16_t fat_ver;
    uint32_t cluster_root;
    uint16_t fat_info;
    uint16_t backup_sector;              /* Fixed */
    uint8_t res[12];
    uint8_t drive_number;
    uint8_t res2;
    uint8_t signature;
    uint32_t volume_id;
    uint8_t label[11];
    uint8_t sysid[8];                    /* Fixed - rename */
                                         /* Remove boot code and sig as not part of BPB */
}__attribute__((packed));

struct DirectoryEntry
{
    uint8_t file_name[11];               /* Fixed 8 -> 11 */
    uint8_t attributes;
    uint8_t res;
    uint8_t ctimeT;
    uint16_t ctime;
    uint16_t cdate;
    uint16_t last_access;
    uint16_t cluster_number_hi;
    uint16_t lmtime;
    uint16_t lmdate;
    uint16_t cluster_number_lo;
    uint32_t file_size;
}__attribute__((packed));

void read_bpb(uint32_t offset)
{
    struct FATBPB bpb;
    hd_read(offset, FATBPB_SIZE, (uint8_t*)&bpb);
    struct DirectoryEntry drce[FAT32_FILES_PER_DIRECTORY];

    uint32_t fat_start = (offset + bpb.reserved_sectors);
    uint32_t fat_size = bpb.sectors_per_fat;
    /* Fixed these calculations */
    uint32_t start_of_data = fat_start + (fat_size * bpb.fats_number);
    uint32_t start_of_root = start_of_data + ((bpb.cluster_root - 2) * bpb.sectors_per_cluster);

    hd_read(start_of_root,
            FAT32_FILES_PER_DIRECTORY * sizeof(struct DirectoryEntry),
            (uint8_t*)&drce[0]);
    for (int i = 0; i < FAT32_FILES_PER_DIRECTORY; ++i) {
        if (drce[i].file_name[0] == 0x00) {
            kputs("\n-- No more files in the directory --\n");
            break;
        }
        if ((drce[i].attributes & 0x0F) == 0x0F)
            continue;

        /* Horrible solution, I'll fix this later, I promise
         * Fixed: put on stack to avoid writing to readonly data */
        char asd[] = "        \n";
        for (int j = 0; j < 8; ++j) {
            asd[j] = drce[i].file_name[j];
        }
        /* This should print the names of the files that are in the root directory... */
        kputs(asd);
        if ((drce[i].attributes & 0x10) == 0x10)
            continue;
        
        uint32_t fcluster = ((uint32_t)drce[i].cluster_number_hi) << 16 | ((uint32_t)drce[i].cluster_number_lo);
        uint32_t fsect = start_of_data + bpb.sectors_per_cluster * (fcluster - 2);
        uint8_t buff[512];
        hd_read(fsect, 512, buff);
        buff[drce[i].file_size] = '\0';
        kputs("%s", (char*)buff);
    }
}

As well your asd buffer was defined in such a way that you invoke undefined behavior when writing to it. Writing to read only data isn't a good idea. I modified your code to define asd's to be allocated on the stack via []. Since we don't see hd_read we can't tell if that code is buggy and causing other issues.

Thanks, here's the hd_read function, just in case:

Code: Select all

void hd_read(uint32_t lba, unsigned int count, uint8_t* buffer)
{   
    /* Is the disk slave or not? */
    outb(DevReg, hd_is_master ? 0xE0 : 0xF0 | (lba & 0x0F000000 >> 24));
    outb(ErrReg, 0);
    outb(SectCountReg, 1);

    outb(LBAloReg, (lba & 0x00000FF));
    outb(LBAmiReg, (lba & 0x000FF00) >> 8);
    outb(LBAloReg, (lba & 0x0FF0000) >> 16);

    hd_send_command(ATA28_READ);

    for (int cout = 0; cout < count; cout += 2) {
        /* Read our data */
        uint16_t wdata = inw(DataReg);
        buffer[cout] = (wdata & 0x00FF);
        if ((cout + 1) < count)
            /* Write it to the buffer */
            buffer[cout + 1] = (wdata >> 8) & 0x00FF;
    }
}

EDIT: Didn't debugged with QEMU because I don't know how to format an QEMU HDD image with FAT32, and then add files to it. I used VirtualBox, much easier, but I'm having some throubles debugging with it.

[MichaelPetch]

This looks wrong:

Code: Select all

outb(LBAloReg, (lba & 0x0FF0000) >> 16);

That is the same port as the low byte. LBAloReg should be replaced with whatever you have defined for port 0x1f5 .

[deleted8917]

Thanks, now it is working.