Re: [PATCH 1/4] exfat: redefine PBR as boot_sector
I'll make another small patch, OK? No, It make sense to make v3, because you have renamed the variables in boot_sector on this patch. OK. BTW I have a concern about fs_name. The exfat specification says that this field is "EXFAT". I think it's a important field for determining the filesystem. However, in this patch, I gave up checking this field. Because there is no similar check in FATFS. Do you know why Linux FATFS does not check this filed? And, what do you think of checking this field? FATFS has the same field named "oem_name" and whatever is okay for its value. However, in case of exFAT, it is an important field to determine filesystem. I think it would be better to check this field for exFAT-fs. Would you like to contribute new patch for checking it? I already have the code, so I'll add it to [PATCH 2/4 v3]. BR
RE: [PATCH 1/4] exfat: redefine PBR as boot_sector
> > [snip]
> >> +/* EXFAT: Main and Backup Boot Sector (512 bytes) */ struct
> >> +boot_sector {
> >> + __u8jmp_boot[BOOTSEC_JUMP_BOOT_LEN];
> >> + __u8oem_name[BOOTSEC_OEM_NAME_LEN];
> >
> > According to the exFAT specification, fs_name and BOOTSEC_FS_NAME_LEN
> > look better.
>
> Oops.
> I sent v2 patches, before I noticed this comment,
>
> I'll make another small patch, OK?
No, It make sense to make v3, because you have renamed the variables in
boot_sector on this patch.
> BTW
> I have a concern about fs_name.
> The exfat specification says that this field is "EXFAT".
>
> I think it's a important field for determining the filesystem.
> However, in this patch, I gave up checking this field.
> Because there is no similar check in FATFS.
> Do you know why Linux FATFS does not check this filed?
> And, what do you think of checking this field?
FATFS has the same field named "oem_name" and whatever is okay for its value.
However, in case of exFAT, it is an important field to determine filesystem.
I think it would be better to check this field for exFAT-fs.
Would you like to contribute new patch for checking it?
>
> BR
Re: [PATCH 1/4] exfat: redefine PBR as boot_sector
[snip]
+/* EXFAT: Main and Backup Boot Sector (512 bytes) */ struct boot_sector
+{
+ __u8jmp_boot[BOOTSEC_JUMP_BOOT_LEN];
+ __u8oem_name[BOOTSEC_OEM_NAME_LEN];
According to the exFAT specification, fs_name and BOOTSEC_FS_NAME_LEN look
better.
Oops.
I sent v2 patches, before I noticed this comment,
I'll make another small patch, OK?
BTW
I have a concern about fs_name.
The exfat specification says that this field is "EXFAT".
I think it's a important field for determining the filesystem.
However, in this patch, I gave up checking this field.
Because there is no similar check in FATFS.
Do you know why Linux FATFS does not check this filed?
And, what do you think of checking this field?
BR
RE: [PATCH 1/4] exfat: redefine PBR as boot_sector
> Aggregate PBR related definitions and redefine as "boot_sector" to comply
> with the exFAT specification.
> And, rename variable names including 'pbr'.
>
> Signed-off-by: Tetsuhiro Kohada
> ---
> fs/exfat/exfat_fs.h | 2 +-
> fs/exfat/exfat_raw.h | 79 +++--
> fs/exfat/super.c | 84 ++--
> 3 files changed, 72 insertions(+), 93 deletions(-)
>
[snip]
> +/* EXFAT: Main and Backup Boot Sector (512 bytes) */ struct boot_sector
> +{
> + __u8jmp_boot[BOOTSEC_JUMP_BOOT_LEN];
> + __u8oem_name[BOOTSEC_OEM_NAME_LEN];
According to the exFAT specification, fs_name and BOOTSEC_FS_NAME_LEN look
better.
> + __u8must_be_zero[BOOTSEC_OLDBPB_LEN];
> + __le64 partition_offset;
> + __le64 vol_length;
> + __le32 fat_offset;
> + __le32 fat_length;
> + __le32 clu_offset;
> + __le32 clu_count;
> + __le32 root_cluster;
> + __le32 vol_serial;
> + __u8fs_revision[2];
> + __le16 vol_flags;
> + __u8sect_size_bits;
> + __u8sect_per_clus_bits;
> + __u8num_fats;
> + __u8drv_sel;
> + __u8percent_in_use;
> + __u8reserved[7];
> + __u8boot_code[390];
> + __le16 signature;
> } __packed;
