OSDN Git Service

mac clone: avoid failed assertion for sector size > 512
authorJim Meyering <meyering@redhat.com>
Sat, 26 Jul 2008 19:15:05 +0000 (21:15 +0200)
committerJim Meyering <meyering@redhat.com>
Fri, 24 Jul 2009 13:04:42 +0000 (15:04 +0200)
* libparted/labels/mac.c (mac_duplicate): Do not assume that
the first sector of the partition-map partition is "1".

Running this: cd libparted/tests &&
PARTED_SECTOR_SIZE=1024 CK_FORK=no valgrind ./label
would result in a failed assertion:

  ped_disk_remove_partition (...) at disk.c:1897
  1897  PED_ASSERT (part->part_list == NULL, goto error);

At first glance, it looked like the free(part) in
mac_partition_destroy was the culprit. And removing that free
does indeed avoid the failed assertion.  However, not only does
the name "mac_partition_destroy" imply/require the "free", but
all other *_partition_destroy functions free their PART parameter,
so removing this free statement cannot be the solution.

Another possibility is that mac_duplicate's use of
ped_disk_remove_partition is in error.  Removing it appears
to solve the problem, but I don't know why that code is removing
the partition-map partition manually, so can't safely remove
such a thing.

The real problem is that with >512-byte sectors, the "1" in this
code from mac_duplicate is wrong:

  partition_map = ped_disk_get_partition_by_sector (new_disk, 1);
  PED_ASSERT (partition_map != NULL, return 0);
  ped_disk_remove_partition (new_disk, partition_map);

E.g., with 1024-byte sectors, the desired partition-map
partition starts at sector 2, so the above gets the tiny
metadata partition in the gap.  Calling ped_disk_remove_partition
to remove a metadata partition provokes some nasty corruption.
The desired first-sector number is old_mac_data->ghost_size, not "1".

libparted/labels/mac.c

index fd507f5..bdbb2cc 100644 (file)
@@ -314,7 +314,6 @@ mac_duplicate (const PedDisk* disk)
        PedDisk*        new_disk;
        MacDiskData*    new_mac_data;
        MacDiskData*    old_mac_data = (MacDiskData*) disk->disk_specific;
-       PedPartition*   partition_map;
 
        new_disk = ped_disk_new_fresh (disk->dev, &mac_disk_type);
        if (!new_disk)
@@ -325,8 +324,18 @@ mac_duplicate (const PedDisk* disk)
        /* remove the partition map partition - it will be duplicated
         * later.
         */
-       partition_map = ped_disk_get_partition_by_sector (new_disk, 1);
+       PedSector first_part_map_sector = old_mac_data->ghost_size;
+       PedPartition *partition_map
+         = ped_disk_get_partition_by_sector (new_disk, first_part_map_sector);
        PED_ASSERT (partition_map != NULL, return 0);
+
+       /* ped_disk_remove_partition may be used only to delete a "normal"
+          partition.  Trying to delete at least "freespace" or "metadata"
+          partitions leads to a violation of assumptions in
+          ped_disk_remove_partition, since it calls _disk_push_update_mode,
+          which destroys all "freespace" and "metadata" partitions, and
+          depends on that destruction not freeing its PART parameter.  */
+       PED_ASSERT (partition_map->type == PED_PARTITION_NORMAL, return 0);
        ped_disk_remove_partition (new_disk, partition_map);
 
        /* ugly, but C is ugly :p */