[OpenBIOS] [PATCH] Fix some unaligned memory accesses

Aurelien Jarno aurelien at aurel32.net
Sun Apr 1 22:05:04 CEST 2007


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.

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.

Regards,
Aurelien


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



More information about the OpenBIOS mailing list