diff mbox

[06/13] vvfat: fix field names in FAT12/FAT16 boot sector

Message ID 20170515203114.9477-7-hpoussin@reactos.org
State New
Headers show

Commit Message

Hervé Poussineau May 15, 2017, 8:31 p.m. UTC
Specification: "FAT: General overview of on-disk format" v1.03, page 11
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 block/vvfat.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Kevin Wolf May 16, 2017, 2:39 p.m. UTC | #1
Am 15.05.2017 um 22:31 hat Hervé Poussineau geschrieben:
> Specification: "FAT: General overview of on-disk format" v1.03, page 11
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  block/vvfat.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vvfat.c b/block/vvfat.c
> index f60d2a3889..348cffe1c4 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -218,10 +218,12 @@ typedef struct bootsector_t {
>      union {
>          struct {
>              uint8_t drive_number;
> -            uint8_t current_head;
> +            uint8_t reserved1;
>              uint8_t signature;
>              uint32_t id;
>              uint8_t volume_label[11];
> +            uint8_t fat_type[8];
> +            uint8_t ignored[0x1c0];
>          } QEMU_PACKED fat16;
>          struct {
>              uint32_t sectors_per_fat;
> @@ -233,8 +235,6 @@ typedef struct bootsector_t {
>              uint16_t ignored;
>          } QEMU_PACKED fat32;
>      } u;
> -    uint8_t fat_type[8];
> -    uint8_t ignored[0x1c0];
>      uint8_t magic[2];
>  } QEMU_PACKED bootsector_t;

At least, this makes it clear that .fat16 and .fat32 aren't the same
length. But maybe it would be cleaner to have a third union member
uint8_t bytes[0x1da] (if I calculated correctly) instead of relying on
the .fat16 branch to extend the space for .fat32?

Kevin
Hervé Poussineau May 17, 2017, 5:28 a.m. UTC | #2
Le 16/05/2017 à 16:39, Kevin Wolf a écrit :
> Am 15.05.2017 um 22:31 hat Hervé Poussineau geschrieben:
>> Specification: "FAT: General overview of on-disk format" v1.03, page 11
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>  block/vvfat.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/block/vvfat.c b/block/vvfat.c
>> index f60d2a3889..348cffe1c4 100644
>> --- a/block/vvfat.c
>> +++ b/block/vvfat.c
>> @@ -218,10 +218,12 @@ typedef struct bootsector_t {
>>      union {
>>          struct {
>>              uint8_t drive_number;
>> -            uint8_t current_head;
>> +            uint8_t reserved1;
>>              uint8_t signature;
>>              uint32_t id;
>>              uint8_t volume_label[11];
>> +            uint8_t fat_type[8];
>> +            uint8_t ignored[0x1c0];
>>          } QEMU_PACKED fat16;
>>          struct {
>>              uint32_t sectors_per_fat;
>> @@ -233,8 +235,6 @@ typedef struct bootsector_t {
>>              uint16_t ignored;
>>          } QEMU_PACKED fat32;
>>      } u;
>> -    uint8_t fat_type[8];
>> -    uint8_t ignored[0x1c0];
>>      uint8_t magic[2];
>>  } QEMU_PACKED bootsector_t;
>
> At least, this makes it clear that .fat16 and .fat32 aren't the same
> length. But maybe it would be cleaner to have a third union member
> uint8_t bytes[0x1da] (if I calculated correctly) instead of relying on
> the .fat16 branch to extend the space for .fat32?

I will also update the .fat32 bootsector to match specification in the same patch.
So, both members (.fat16 et .fat32) will have the same size.

BTW, FAT32 doesn't work at all, so that's not really important :)

Hervé
diff mbox

Patch

diff --git a/block/vvfat.c b/block/vvfat.c
index f60d2a3889..348cffe1c4 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -218,10 +218,12 @@  typedef struct bootsector_t {
     union {
         struct {
             uint8_t drive_number;
-            uint8_t current_head;
+            uint8_t reserved1;
             uint8_t signature;
             uint32_t id;
             uint8_t volume_label[11];
+            uint8_t fat_type[8];
+            uint8_t ignored[0x1c0];
         } QEMU_PACKED fat16;
         struct {
             uint32_t sectors_per_fat;
@@ -233,8 +235,6 @@  typedef struct bootsector_t {
             uint16_t ignored;
         } QEMU_PACKED fat32;
     } u;
-    uint8_t fat_type[8];
-    uint8_t ignored[0x1c0];
     uint8_t magic[2];
 } QEMU_PACKED bootsector_t;
 
@@ -972,13 +972,13 @@  static int init_directories(BDRVVVFATState* s,
 
     /* LATER TODO: if FAT32, this is wrong */
     bootsector->u.fat16.drive_number = s->offset_to_bootsector == 0 ? 0 : 0x80;
-    bootsector->u.fat16.current_head=0;
     bootsector->u.fat16.signature=0x29;
     bootsector->u.fat16.id=cpu_to_le32(0xfabe1afd);
 
     memcpy(bootsector->u.fat16.volume_label, s->volume_label,
            sizeof(bootsector->u.fat16.volume_label));
-    memcpy(bootsector->fat_type,(s->fat_type==12?"FAT12   ":s->fat_type==16?"FAT16   ":"FAT32   "),8);
+    memcpy(bootsector->u.fat16.fat_type,
+           s->fat_type == 12 ? "FAT12   " : "FAT16   ", 8);
     bootsector->magic[0]=0x55; bootsector->magic[1]=0xaa;
 
     return 0;