* disk/mdraid_linux.c (struct grub_raid_super_1x): Remove

__attribute__ ((packed)), leaving a comment.
(grub_mdraid_detect): Split out 0.9 and 1.x detection to ...
(grub_mdraid_detect_09): ... here and ...
(grub_mdraid_detect_1x): ... here.
* disk/raid.c (insert_array): Check for grub_xasprintf returning
NULL.

diff --git a/ChangeLog.raid b/ChangeLog.raid
index a66bd69052c01df3dec9dbf816b04861fb97107a..d91f584c04d74f2205cfc7d44a5ffacf91a8879c 100644 (file)
--- a/ChangeLog.raid
+++ b/ChangeLog.raid
@@ -1,3 +1,13 @@
+2010-07-19  Colin Watson  <cjwatson@ubuntu.com>
+
+       * disk/mdraid_linux.c (struct grub_raid_super_1x): Remove
+       __attribute__ ((packed)), leaving a comment.
+       (grub_mdraid_detect): Split out 0.9 and 1.x detection to ...
+       (grub_mdraid_detect_09): ... here and ...
+       (grub_mdraid_detect_1x): ... here.
+       * disk/raid.c (insert_array): Check for grub_xasprintf returning
+       NULL.
+
 2010-07-18  Colin Watson  <cjwatson@ubuntu.com>
 
        * disk/dmraid_nvidia.c (grub_dmraid_nv_detect): Add start_sector

diff --git a/disk/mdraid_linux.c b/disk/mdraid_linux.c
index b97e458ce618ed1cd662085a3b38a6b76f3addfd..b1199c6efc21d7be4a78f6d5e16ca45861e6ddce 100644 (file)
--- a/disk/mdraid_linux.c
+++ b/disk/mdraid_linux.c
@@ -224,178 +224,189 @@ struct grub_raid_super_1x
    * have a meaningful role.
    */
   grub_uint16_t dev_roles[0];  /* Role in array, or 0xffff for a spare, or 0xfffe for faulty.  */
-} __attribute__ ((packed));
+};
+/* Could be __attribute__ ((packed)), but since all members in this struct
+   are already appropriately aligned, we can omit this and avoid suboptimal
+   assembly in some cases.  */
 
 #define WriteMostly1    1      /* Mask for writemostly flag in above devflags.  */
 
 static grub_err_t
-grub_mdraid_detect (grub_disk_t disk, struct grub_raid_array *array,
-                   grub_disk_addr_t *start_sector)
+grub_mdraid_detect_09 (grub_disk_addr_t sector,
+                      struct grub_raid_super_09 *sb,
+                      struct grub_raid_array *array,
+                      grub_disk_addr_t *start_sector)
 {
-  grub_disk_addr_t sector;
-  grub_uint64_t size, sb_size;
-  struct grub_raid_super_09 sb;
-  struct grub_raid_super_1x *sb_1x;
   grub_uint32_t *uuid;
-  grub_uint8_t minor_version;
-
-  /* The sector where the mdraid 0.90 superblock is stored, if available.  */
-  size = grub_disk_get_size (disk);
-  sector = NEW_SIZE_SECTORS (size);
-
-  if (grub_disk_read (disk, sector, 0, SB_BYTES, &sb))
-    return grub_errno;
-
-  /* Look whether there is a mdraid 0.90 superblock.  */
-  if (sb.md_magic == SB_MAGIC)
-    goto superblock_0_90;
-
-  /* Check for an 1.x superblock.
-   * It's always aligned to a 4K boundary
-   * and depending on the minor version it can be:
-   * 0: At least 8K, but less than 12K, from end of device
-   * 1: At start of device
-   * 2: 4K from start of device.
-   */
-
-  sb_1x = grub_malloc (sizeof (struct grub_raid_super_1x));
-  if (!sb_1x)
-    return grub_errno;
-
-  for (minor_version = 0; minor_version < 3; ++minor_version)
-    {
-      switch (minor_version)
-       {
-       case 0:
-         sector = (size - 8 * 2) & ~(4 * 2 - 1);
-         break;
-       case 1:
-         sector = 0;
-         break;
-       case 2:
-         sector = 4 * 2;
-         break;
-       }
-
-      if (grub_disk_read
-         (disk, sector, 0, sizeof (struct grub_raid_super_1x), sb_1x))
-       {
-         grub_free (sb_1x);
-         return grub_errno;
-       }
-
-      if (sb_1x->magic == SB_MAGIC)
-       goto superblock_1_x;
-    }
 
-  /* Neither 0.90 nor 1.x.  */
-  if (grub_le_to_cpu32 (sb_1x->magic) != SB_MAGIC)
-    return grub_error (GRUB_ERR_OUT_OF_RANGE, "not raid");
-
-superblock_0_90:
-
-  if (sb.major_version != 0 || sb.minor_version != 90)
+  if (sb->major_version != 0 || sb->minor_version != 90)
     return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
                       "unsupported RAID version: %d.%d",
-                      sb.major_version, sb.minor_version);
+                      sb->major_version, sb->minor_version);
 
   /* FIXME: Check the checksum.  */
 
   /* Multipath.  */
-  if ((int) sb.level == -4)
-    sb.level = 1;
+  if ((int) sb->level == -4)
+    sb->level = 1;
 
-  if (sb.level != 0 && sb.level != 1 && sb.level != 4 &&
-      sb.level != 5 && sb.level != 6 && sb.level != 10)
+  if (sb->level != 0 && sb->level != 1 && sb->level != 4 &&
+      sb->level != 5 && sb->level != 6 && sb->level != 10)
     return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-                      "unsupported RAID level: %d", sb.level);
+                      "unsupported RAID level: %d", sb->level);
 
   array->name = NULL;
-  array->number = sb.md_minor;
-  array->level = sb.level;
-  array->layout = sb.layout;
-  array->total_devs = sb.raid_disks;
-  array->disk_size = (sb.size) ? sb.size * 2 : sector;
-  array->chunk_size = sb.chunk_size >> 9;
-  array->index = sb.this_disk.number;
+  array->number = sb->md_minor;
+  array->level = sb->level;
+  array->layout = sb->layout;
+  array->total_devs = sb->raid_disks;
+  array->disk_size = (sb->size) ? sb->size * 2 : sector;
+  array->chunk_size = sb->chunk_size >> 9;
+  array->index = sb->this_disk.number;
   array->uuid_len = 16;
   array->uuid = grub_malloc (16);
   if (!array->uuid)
       return grub_errno;
 
   uuid = (grub_uint32_t *) array->uuid;
-  uuid[0] = sb.set_uuid0;
-  uuid[1] = sb.set_uuid1;
-  uuid[2] = sb.set_uuid2;
-  uuid[3] = sb.set_uuid3;
+  uuid[0] = sb->set_uuid0;
+  uuid[1] = sb->set_uuid1;
+  uuid[2] = sb->set_uuid2;
+  uuid[3] = sb->set_uuid3;
 
   *start_sector = 0;
 
   return 0;
+}
 
- superblock_1_x:
+static grub_err_t
+grub_mdraid_detect_1x (grub_disk_t disk, grub_disk_addr_t sector,
+                      struct grub_raid_super_1x *sb,
+                      struct grub_raid_array *array,
+                      grub_disk_addr_t *start_sector)
+{
+  grub_uint64_t sb_size;
+  struct grub_raid_super_1x *real_sb;
 
-  if (sb_1x->major_version != 1)
+  if (sb->major_version != 1)
     return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
                       "Unsupported RAID version: %d",
-                      sb_1x->major_version);
+                      sb->major_version);
+
   /* Multipath.  */
-  if ((int) sb_1x->level == -4)
-    sb_1x->level = 1;
+  if ((int) sb->level == -4)
+    sb->level = 1;
+
+  if (sb->level != 0 && sb->level != 1 && sb->level != 4 &&
+      sb->level != 5 && sb->level != 6 && sb->level != 10)
+    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+                      "Unsupported RAID level: %d", sb->level);
 
-  if (sb_1x->level != 0 && sb_1x->level != 1 && sb_1x->level != 4 &&
-      sb_1x->level != 5 && sb_1x->level != 6 && sb_1x->level != 10)
-    {
-      return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-                        "Unsupported RAID level: %d", sb_1x->level);
-      grub_free (sb_1x);
-    }
   /* 1.x superblocks don't have a fixed size on disk.  So we have to
      read it again now that we now the max device count.  */
-  sb_size = sizeof (struct grub_raid_super_1x) + 2 * grub_le_to_cpu32 (sb_1x->max_dev);
-  sb_1x = grub_realloc (sb_1x, sb_size);
-  if (! sb_1x)
+  sb_size = sizeof (struct grub_raid_super_1x) + 2 * grub_le_to_cpu32 (sb->max_dev);
+  real_sb = grub_malloc (sb_size);
+  if (! real_sb)
     return grub_errno;
 
-  if (grub_disk_read (disk, sector, 0, sb_size, sb_1x))
+  if (grub_disk_read (disk, sector, 0, sb_size, real_sb))
     {
-      grub_free (sb_1x);
+      grub_free (real_sb);
       return grub_errno;
     }
 
-  array->name = grub_strdup (sb_1x->set_name);
+  array->name = grub_strdup (real_sb->set_name);
   if (! array->name)
     {
-      grub_free (sb_1x);
+      grub_free (real_sb);
       return grub_errno;
     }
 
   array->number = 0;
-  array->level = grub_le_to_cpu32 (sb_1x->level);
-  array->layout = grub_le_to_cpu32 (sb_1x->layout);
-  array->total_devs = grub_le_to_cpu32 (sb_1x->raid_disks);
-  array->disk_size = grub_le_to_cpu64 (sb_1x->size);
-  array->chunk_size = grub_le_to_cpu32 (sb_1x->chunksize);
-  if (grub_le_to_cpu32 (sb_1x->dev_number) < grub_le_to_cpu32 (sb_1x->max_dev))
-    array->index = grub_le_to_cpu16 (sb_1x->dev_roles[grub_le_to_cpu32 (sb_1x->dev_number)]);
+  array->level = grub_le_to_cpu32 (real_sb->level);
+  array->layout = grub_le_to_cpu32 (real_sb->layout);
+  array->total_devs = grub_le_to_cpu32 (real_sb->raid_disks);
+  array->disk_size = grub_le_to_cpu64 (real_sb->size);
+  array->chunk_size = grub_le_to_cpu32 (real_sb->chunksize);
+  if (grub_le_to_cpu32 (real_sb->dev_number) <
+      grub_le_to_cpu32 (real_sb->max_dev))
+    array->index = grub_le_to_cpu16
+      (real_sb->dev_roles[grub_le_to_cpu32 (real_sb->dev_number)]);
   else
     array->index = 0xffff;  /* disk will be later not used! */
   array->uuid_len = 16;
   array->uuid = grub_malloc (16);
   if (!array->uuid)
     {
-      grub_free (sb_1x);
+      grub_free (real_sb);
       return grub_errno;
     }
 
-  grub_memcpy (array->uuid, sb_1x->set_uuid, 16);
+  grub_memcpy (array->uuid, real_sb->set_uuid, 16);
 
-  *start_sector = sb_1x->data_offset;
+  *start_sector = real_sb->data_offset;
 
-  grub_free (sb_1x);
+  grub_free (real_sb);
   return 0;
 }
 
+static grub_err_t
+grub_mdraid_detect (grub_disk_t disk, struct grub_raid_array *array,
+                   grub_disk_addr_t *start_sector)
+{
+  grub_disk_addr_t sector;
+  grub_uint64_t size;
+  struct grub_raid_super_09 sb_09;
+  struct grub_raid_super_1x sb_1x;
+  grub_uint8_t minor_version;
+
+  /* The sector where the mdraid 0.90 superblock is stored, if available.  */
+  size = grub_disk_get_size (disk);
+  sector = NEW_SIZE_SECTORS (size);
+
+  if (grub_disk_read (disk, sector, 0, SB_BYTES, &sb_09))
+    return grub_errno;
+
+  /* Look whether there is a mdraid 0.90 superblock.  */
+  if (sb_09.md_magic == SB_MAGIC)
+    return grub_mdraid_detect_09 (sector, &sb_09, array, start_sector);
+
+  /* Check for an 1.x superblock.
+   * It's always aligned to a 4K boundary
+   * and depending on the minor version it can be:
+   * 0: At least 8K, but less than 12K, from end of device
+   * 1: At start of device
+   * 2: 4K from start of device.
+   */
+
+  for (minor_version = 0; minor_version < 3; ++minor_version)
+    {
+      switch (minor_version)
+       {
+       case 0:
+         sector = (size - 8 * 2) & ~(4 * 2 - 1);
+         break;
+       case 1:
+         sector = 0;
+         break;
+       case 2:
+         sector = 4 * 2;
+         break;
+       }
+
+      if (grub_disk_read (disk, sector, 0, sizeof (struct grub_raid_super_1x),
+                         &sb_1x))
+       return grub_errno;
+
+      if (sb_1x.magic == SB_MAGIC)
+       return grub_mdraid_detect_1x (disk, sector, &sb_1x, array,
+                                     start_sector);
+    }
+
+  /* Neither 0.90 nor 1.x.  */
+  return grub_error (GRUB_ERR_OUT_OF_RANGE, "not raid");
+}
+
 static struct grub_raid grub_mdraid_dev = {
   .name = "mdraid",
   .detect = grub_mdraid_detect,

diff --git a/disk/raid.c b/disk/raid.c
index 30d4b7ac4329adb5aa839c7b014fc00033430c00..2f70e256f306237187ecf782f6ada8c32c0c3f69 100644 (file)
--- a/disk/raid.c
+++ b/disk/raid.c
@@ -564,13 +564,31 @@ insert_array (grub_disk_t disk, struct grub_raid_array *new_array,
       /* mdraid 1.x superblocks have only a name stored not a number.
         Use it directly as GRUB device.  */
       if (! array->name)
-       array->name = grub_xasprintf ("md%d", array->number);
+       {
+         array->name = grub_xasprintf ("md%d", array->number);
+         if (! array->name)
+           {
+             grub_free (array->uuid);
+             grub_free (array);
+
+             return grub_errno;
+           }
+       }
       else
        {
          /* Strip off the homehost if present.  */
          char *colon = grub_strchr (array->name, ':');
          char *new_name = grub_xasprintf ("md/%s",
                                           colon ? colon + 1 : array->name);
+
+         if (! new_name)
+           {
+             grub_free (array->uuid);
+             grub_free (array);
+
+             return grub_errno;
+           }
+
          grub_free (array->name);
          array->name = new_name;
        }