[OpenBIOS] [PATCH] Fix some unaligned memory accesses
Stefan Reinauer
stepan at coresystems.de
Sun Apr 15 02:23:53 CEST 2007
* Aurelien Jarno <aurelien at aurel32.net> [070401 22:05]:
> Hi all,
>
> The current SVN version currently fails to work on machines that have
> strict alignment requirements, for example the SPARC target. This is due
> to the way the struct fat_bpb in fs/grubfs/fat.h is defined and accessed
> (through FAT_CVT_U16) to avoid padding. Some 16 bits fields like
> bytes_per_sect are thus accessed unaligned.
Why is this causing trouble? bytes_per_sect was defined as an array of
u8, which you would not need to access aligned anyways. Is gcc doing
some assumptions here?
> The patch below takes the same approach as for other structures in
> openbios, it declares the structure as packed, and lets GCC do the right
> things to access those unaligned field.
Thanks, and sorry the patch is taking so long to get in. It will be
checked in in a couple of days.
> Index: fs/grubfs/fsys_fat.c
> ===================================================================
> --- fs/grubfs/fsys_fat.c (révision 116)
> +++ fs/grubfs/fsys_fat.c (copie de travail)
> @@ -75,23 +75,23 @@
> if (bpb.sects_per_clust == 0)
> return 0;
>
> - FAT_SUPER->sectsize_bits = log2 (FAT_CVT_U16 (bpb.bytes_per_sect));
> + FAT_SUPER->sectsize_bits = log2 (bpb.bytes_per_sect);
> FAT_SUPER->clustsize_bits
> = FAT_SUPER->sectsize_bits + log2 (bpb.sects_per_clust);
>
> /* Fill in info about super block */
> - FAT_SUPER->num_sectors = FAT_CVT_U16 (bpb.short_sectors)
> - ? FAT_CVT_U16 (bpb.short_sectors) : bpb.long_sectors;
> + FAT_SUPER->num_sectors = bpb.short_sectors
> + ? bpb.short_sectors : bpb.long_sectors;
>
> /* FAT offset and length */
> - FAT_SUPER->fat_offset = FAT_CVT_U16 (bpb.reserved_sects);
> + FAT_SUPER->fat_offset = bpb.reserved_sects;
> FAT_SUPER->fat_length =
> bpb.fat_length ? bpb.fat_length : bpb.fat32_length;
>
> /* Rootdir offset and length for FAT12/16 */
> FAT_SUPER->root_offset =
> FAT_SUPER->fat_offset + bpb.num_fats * FAT_SUPER->fat_length;
> - FAT_SUPER->root_max = FAT_DIRENTRY_LENGTH * FAT_CVT_U16(bpb.dir_entries);
> + FAT_SUPER->root_max = FAT_DIRENTRY_LENGTH * bpb.dir_entries;
>
> /* Data offset and number of clusters */
> FAT_SUPER->data_offset =
> @@ -105,7 +105,7 @@
> if (!bpb.fat_length)
> {
> /* This is a FAT32 */
> - if (FAT_CVT_U16(bpb.dir_entries))
> + if (bpb.dir_entries)
> return 0;
>
> if (bpb.flags & 0x0080)
> @@ -144,8 +144,8 @@
>
> /* Now do some sanity checks */
>
> - if (FAT_CVT_U16(bpb.bytes_per_sect) != (1 << FAT_SUPER->sectsize_bits)
> - || FAT_CVT_U16(bpb.bytes_per_sect) != SECTOR_SIZE
> + if (bpb.bytes_per_sect != (1 << FAT_SUPER->sectsize_bits)
> + || bpb.bytes_per_sect != SECTOR_SIZE
> || bpb.sects_per_clust != (1 << (FAT_SUPER->clustsize_bits
> - FAT_SUPER->sectsize_bits))
> || FAT_SUPER->num_clust <= 2
> Index: fs/grubfs/fat.h
> ===================================================================
> --- fs/grubfs/fat.h (révision 116)
> +++ fs/grubfs/fat.h (copie de travail)
> @@ -37,12 +37,12 @@
> __s8 ignored[3]; /* Boot strap short or near jump */
> __s8 system_id[8]; /* Name - can be used to special case
> partition manager volumes */
> - __u8 bytes_per_sect[2]; /* bytes per logical sector */
> + __u16 bytes_per_sect; /* bytes per logical sector */
> __u8 sects_per_clust;/* sectors/cluster */
> - __u8 reserved_sects[2]; /* reserved sectors */
> + __u16 reserved_sects; /* reserved sectors */
> __u8 num_fats; /* number of FATs */
> - __u8 dir_entries[2]; /* root directory entries */
> - __u8 short_sectors[2]; /* number of sectors */
> + __u16 dir_entries; /* root directory entries */
> + __u16 short_sectors; /* number of sectors */
> __u8 media; /* media code (unused) */
> __u16 fat_length; /* sectors/FAT */
> __u16 secs_track; /* sectors per track */
> @@ -53,15 +53,13 @@
> /* The following fields are only used by FAT32 */
> __u32 fat32_length; /* sectors/FAT */
> __u16 flags; /* bit 8: fat mirroring, low 4: active fat */
> - __u8 version[2]; /* major, minor filesystem version */
> + __u16 version; /* major, minor filesystem version */
> __u32 root_cluster; /* first cluster in root directory */
> __u16 info_sector; /* filesystem info sector */
> __u16 backup_boot; /* backup boot sector */
> __u16 reserved2[6]; /* Unused */
> -};
> +} __attribute__ ((packed));
>
> -#define FAT_CVT_U16(bytarr) (* (__u16*)(bytarr))
> -
> /*
> * Defines how to differentiate a 12-bit and 16-bit FAT.
> */
>
> --
> .''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
> : :' : Debian developer | Electrical Engineer
> `. `' aurel32 at debian.org | aurelien at aurel32.net
> `- people.debian.org/~aurel32 | www.aurel32.net
>
> --
> OpenBIOS http://openbios.org/
> Mailinglist: http://lists.openbios.org/mailman/listinfo
> Free your System - May the Forth be with you
>
--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
More information about the OpenBIOS
mailing list