On Friday 20 December 2019 01:24:07 Namjae Jeon wrote:
> This adds in-memory and on-disk structures and headers.
>
> Signed-off-by: Namjae Jeon <namjae.jeon@xxxxxxxxxxx>
> Signed-off-by: Sungjong Seo <sj1557.seo@xxxxxxxxxxx>
> ---
> fs/exfat/exfat_fs.h | 559 +++++++++++++++++++++++++++++++++++++++++++
> fs/exfat/exfat_raw.h | 202 ++++++++++++++++
> 2 files changed, 761 insertions(+)
> create mode 100644 fs/exfat/exfat_fs.h
> create mode 100644 fs/exfat/exfat_raw.h
...
> diff --git a/fs/exfat/exfat_raw.h b/fs/exfat/exfat_raw.h
> new file mode 100644
> index 000000000000..a3ccac835993
> --- /dev/null
> +++ b/fs/exfat/exfat_raw.h
...
> +/* file attributes */
> +#define ATTR_READONLY 0x0001
> +#define ATTR_HIDDEN 0x0002
> +#define ATTR_SYSTEM 0x0004
> +#define ATTR_VOLUME 0x0008
> +#define ATTR_SUBDIR 0x0010
> +#define ATTR_ARCHIVE 0x0020
> +#define ATTR_EXTEND (ATTR_READONLY | ATTR_HIDDEN | ATTR_SYSTEM | \
> + ATTR_VOLUME) /* 0x000F */
> +
> +#define ATTR_EXTEND_MASK (ATTR_EXTEND | ATTR_SUBDIR | ATTR_ARCHIVE)
> +#define ATTR_RWMASK (ATTR_HIDDEN | ATTR_SYSTEM | ATTR_VOLUME | \
> + ATTR_SUBDIR | ATTR_ARCHIVE)
> +
> +#define ATTR_READONLY_LE cpu_to_le16(0x0001)
> +#define ATTR_HIDDEN_LE cpu_to_le16(0x0002)
> +#define ATTR_SYSTEM_LE cpu_to_le16(0x0004)
> +#define ATTR_VOLUME_LE cpu_to_le16(0x0008)
> +#define ATTR_SUBDIR_LE cpu_to_le16(0x0010)
> +#define ATTR_ARCHIVE_LE cpu_to_le16(0x0020)
Hello!
This looks like copy-paste code from /* file attributes */ section
above. What about at least making these macro definitions as?
#define ATTR_READONLY_LE cpu_to_le16(ATTR_READONLY)
#define ATTR_HIDDEN_LE cpu_to_le16(ATTR_HIDDEN)
...
But main question is, are these _LE definitions needed at all?
Looking at the whole patch series and only ATTR_SUBDIR_LE and
ATTR_ARCHIVE_LE are used.
Is not it better to use cpu_to_le16(ATTR_READONLY) directly in code and
do not define duplicate ATTR_READONLY_LE macro at all?
> +
> +#define JUMP_BOOT_LEN 3
> +#define OEM_NAME_LEN 8
> +#define MUST_BE_ZERO_LEN 53
> +#define EXFAT_FILE_NAME_LEN 15
> +
> +/* EXFAT BIOS parameter block (64 bytes) */
> +struct bpb64 {
> + __u8 jmp_boot[JUMP_BOOT_LEN];
> + __u8 oem_name[OEM_NAME_LEN];
> + __u8 res_zero[MUST_BE_ZERO_LEN];
> +};
> +
> +/* EXFAT EXTEND BIOS parameter block (56 bytes) */
> +struct bsx64 {
> + __le64 vol_offset;
> + __le64 vol_length;
> + __le32 fat_offset;
> + __le32 fat_length;
> + __le32 clu_offset;
> + __le32 clu_count;
> + __le32 root_cluster;
> + __le32 vol_serial;
> + __u8 fs_version[2];
> + __le16 vol_flags;
> + __u8 sect_size_bits;
> + __u8 sect_per_clus_bits;
> + __u8 num_fats;
> + __u8 phy_drv_no;
> + __u8 perc_in_use;
> + __u8 reserved2[7];
> +};
Should not be this structure marked as packed? Also those two below.
> +/* EXFAT PBR[BPB+BSX] (120 bytes) */
> +struct pbr64 {
> + struct bpb64 bpb;
> + struct bsx64 bsx;
> +};
> +
> +/* Common PBR[Partition Boot Record] (512 bytes) */
> +struct pbr {
> + union {
> + __u8 raw[64];
> + struct bpb64 f64;
> + } bpb;
> + union {
> + __u8 raw[56];
> + struct bsx64 f64;
> + } bsx;
> + __u8 boot_code[390];
> + __le16 signature;
> +};
--
Pali Rohár
pali.rohar@xxxxxxxxx
Attachment:
signature.asc
Description: PGP signature
