This isn't a review request, because it's not ready for commit yet,
but rather a heads-up for what's on the way.
I'm making it so all partition types work with a sector size of 2048-bytes.
Then, things should work with an arbitrary sector size,
as long as it's a multiple of 512 bytes.
Why? Well, such disks are becoming more and more common, and
the existing code is very brittle. There are lots of hard-coded
512-byte sector size assumptions (and worse).
So far, the following have had trouble and I've fixed them:
amiga, bsd, gpt
Next in line is "mac".
Another example of something that will change:
all of the added read_sector functions will be factored
out in the final patch.
Since I'm running valgrind as I go, I'm also fixing leaks as
I find them. Many of the changes below are leak fixes.
They'll end up being in a separate delta, eventually.
diff --git a/libparted/labels/bsd.c b/libparted/labels/bsd.c
index 747a9c5..26a8b60 100644
--- a/libparted/labels/bsd.c
+++ b/libparted/labels/bsd.c
@@ -22,6 +22,7 @@
#include <config.h>
+#include <stdbool.h>
#include <parted/parted.h>
#include <parted/debug.h>
#include <parted/endian.h>
@@ -141,30 +142,45 @@ alpha_bootblock_checksum (char *boot) {
dp[63] = sum;
}
+/* FIXME: factor out this function: copied from dos.c
+ Read sector, SECTOR_NUM (which has length DEV->sector_size) into malloc'd
+ storage. If the read fails, free the memory and return zero without
+ modifying *BUF. Otherwise, set *BUF to the new buffer and return 1. */
+static int
+read_sector (const PedDevice *dev, PedSector sector_num, char **buf)
+{
+ char *b = ped_malloc (dev->sector_size);
+ PED_ASSERT (b != NULL, return 0);
+ if (!ped_device_read (dev, b, sector_num, 1)) {
+ ped_free (b);
+ return 0;
+ }
+ *buf = b;
+ return 1;
+}
static int
bsd_probe (const PedDevice *dev)
{
- char boot[512];
- BSDRawLabel *label;
+ BSDRawLabel *partition;
PED_ASSERT (dev != NULL, return 0);
- if (dev->sector_size != 512)
+ if (dev->sector_size < 512)
return 0;
- if (!ped_device_read (dev, boot, 0, 1))
+ char *label;
+ if (!read_sector (dev, 0, &label))
return 0;
- label = (BSDRawLabel *) (boot + BSD_LABEL_OFFSET);
+ partition = (BSDRawLabel *) (label + BSD_LABEL_OFFSET);
- alpha_bootblock_checksum(boot);
-
- /* check magic */
- if (PED_LE32_TO_CPU (label->d_magic) != BSD_DISKMAGIC)
- return 0;
+ alpha_bootblock_checksum(label);
- return 1;
+ /* check magic */
+ bool found = PED_LE32_TO_CPU (partition->d_magic) == BSD_DISKMAGIC;
+ free (label);
+ return found;
}
static PedDisk*
@@ -248,13 +264,12 @@ bsd_free (PedDisk* disk)
static int
bsd_clobber (PedDevice* dev)
{
- char boot [512];
- BSDRawLabel* label = (BSDRawLabel *) (boot + BSD_LABEL_OFFSET);
-
- if (!ped_device_read (dev, boot, 0, 1))
+ char *label;
+ if (!read_sector (dev, 0, &label))
return 0;
- label->d_magic = 0;
- return ped_device_write (dev, (void*) boot, 0, 1);
+ BSDRawLabel *rawlabel = (BSDRawLabel *) (label + BSD_LABEL_OFFSET);
+ rawlabel->d_magic = 0;
+ return ped_device_write (dev, label, 0, 1);
}
#endif /* !DISCOVER_ONLY */
@@ -267,8 +282,13 @@ bsd_read (PedDisk* disk)
ped_disk_delete_all (disk);
- if (!ped_device_read (disk->dev, bsd_specific->boot_code, 0, 1))
- goto error;
+ char *s0;
+ if (!read_sector (disk->dev, 0, &s0))
+ return 0;
+
+ memcpy (bsd_specific->boot_code, s0, sizeof (bsd_specific->boot_code));
+ free (s0);
+
label = (BSDRawLabel *) (bsd_specific->boot_code + BSD_LABEL_OFFSET);
for (i = 1; i <= BSD_MAXPARTITIONS; i++) {
diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
index 86c31c7..0b874b1 100644
--- a/libparted/labels/gpt.c
+++ b/libparted/labels/gpt.c
@@ -254,6 +254,23 @@ typedef struct _GPTPartitionData {
static PedDiskType gpt_disk_type;
+/* FIXME: factor out this function: copied from dos.c
+ Read sector, SECTOR_NUM (which has length DEV->sector_size) into malloc'd
+ storage. If the read fails, free the memory and return zero without
+ modifying *BUF. Otherwise, set *BUF to the new buffer and return 1. */
+static int
+read_sector (const PedDevice *dev, PedSector sector_num, char **buf)
+{
+ char *b = ped_malloc (dev->sector_size);
+ PED_ASSERT (b != NULL, return 0);
+ if (!ped_device_read (dev, b, sector_num, 1)) {
+ ped_free (b);
+ return 0;
+ }
+ *buf = b;
+ return 1;
+}
+
static inline uint32_t
pth_get_size (const PedDevice* dev)
{
@@ -425,7 +442,6 @@ gpt_probe (const PedDevice * dev)
{
GuidPartitionTableHeader_t* gpt = NULL;
uint8_t* pth_raw = ped_malloc (pth_get_size (dev));
- LegacyMBR_t legacy_mbr;
int gpt_sig_found = 0;
PED_ASSERT (dev != NULL, return 0);
@@ -446,26 +462,31 @@ gpt_probe (const PedDevice * dev)
if (!gpt_sig_found)
return 0;
- if (ped_device_read(dev, &legacy_mbr, 0, GPT_HEADER_SECTORS)) {
- if (!_pmbr_is_valid (&legacy_mbr)) {
- int ex_status = ped_exception_throw (
- PED_EXCEPTION_WARNING,
- PED_EXCEPTION_YES_NO,
- _("%s contains GPT signatures, indicating that it has "
- "a GPT table. However, it does not have a valid "
- "fake msdos partition table, as it should. Perhaps "
- "it was corrupted -- possibly by a program that "
- "doesn't understand GPT partition tables. Or "
- "perhaps you deleted the GPT table, and are now "
- "using an msdos partition table. Is this a GPT "
- "partition table?"),
- dev->path);
- if (ex_status == PED_EXCEPTION_NO)
- return 0;
- }
+
+ char *label;
+ if (!read_sector (dev, 0, &label))
+ return 0;
+
+ int ok = 1;
+ if (!_pmbr_is_valid (label)) {
+ int ex_status = ped_exception_throw (
+ PED_EXCEPTION_WARNING,
+ PED_EXCEPTION_YES_NO,
+ _("%s contains GPT signatures, indicating that it has "
+ "a GPT table. However, it does not have a valid "
+ "fake msdos partition table, as it should. Perhaps "
+ "it was corrupted -- possibly by a program that "
+ "doesn't understand GPT partition tables. Or "
+ "perhaps you deleted the GPT table, and are now "
+ "using an msdos partition table. Is this a GPT "
+ "partition table?"),
+ dev->path);
+ if (ex_status == PED_EXCEPTION_NO)
+ ok = 0;
}
- return 1;
+ ped_free (label);
+ return ok;
}
#ifndef DISCOVER_ONLY
@@ -828,8 +849,10 @@ gpt_read (PedDisk * disk)
"should be. This might mean that another operating system "
"believes the disk is smaller. Fix, by moving the backup "
"to the end (and removing the old backup)?"))
- == PED_EXCEPTION_CANCEL)
+ == PED_EXCEPTION_CANCEL) {
+ free (zeros);
goto error_free_gpt;
+ }
write_back = 1;
memset (zeros, 0, disk->dev->sector_size);
@@ -837,6 +860,7 @@ gpt_read (PedDisk * disk)
PED_LE64_TO_CPU (gpt->AlternateLBA),
1);
#endif /* !DISCOVER_ONLY */
+ free (zeros);
}
} else { /* primary GPT *not* ok */
int alternate_ok = 0;
@@ -914,6 +938,7 @@ gpt_read (PedDisk * disk)
ped_disk_commit_to_dev (disk);
#endif
+ pth_free (gpt);
return 1;
error_delete_all:
@@ -931,22 +956,26 @@ error:
static int
_write_pmbr (PedDevice * dev)
{
- LegacyMBR_t pmbr;
-
- memset(&pmbr, 0, sizeof(pmbr));
- pmbr.Signature = PED_CPU_TO_LE16(MSDOS_MBR_SIGNATURE);
- pmbr.PartitionRecord[0].OSType = EFI_PMBR_OSTYPE_EFI;
- pmbr.PartitionRecord[0].StartSector = 1;
- pmbr.PartitionRecord[0].EndHead = 0xFE;
- pmbr.PartitionRecord[0].EndSector = 0xFF;
- pmbr.PartitionRecord[0].EndTrack = 0xFF;
- pmbr.PartitionRecord[0].StartingLBA = PED_CPU_TO_LE32(1);
+ size_t buf_len = pth_get_size (dev);
+ LegacyMBR_t *pmbr = ped_malloc (buf_len);
+
+ memset(pmbr, 0, buf_len);
+ pmbr->Signature = PED_CPU_TO_LE16(MSDOS_MBR_SIGNATURE);
+ pmbr->PartitionRecord[0].OSType = EFI_PMBR_OSTYPE_EFI;
+ pmbr->PartitionRecord[0].StartSector = 1;
+ pmbr->PartitionRecord[0].EndHead = 0xFE;
+ pmbr->PartitionRecord[0].EndSector = 0xFF;
+ pmbr->PartitionRecord[0].EndTrack = 0xFF;
+ pmbr->PartitionRecord[0].StartingLBA = PED_CPU_TO_LE32(1);
if ((dev->length - 1ULL) > 0xFFFFFFFFULL)
- pmbr.PartitionRecord[0].SizeInLBA = PED_CPU_TO_LE32(0xFFFFFFFF);
+ pmbr->PartitionRecord[0].SizeInLBA =
PED_CPU_TO_LE32(0xFFFFFFFF);
else
- pmbr.PartitionRecord[0].SizeInLBA = PED_CPU_TO_LE32(dev->length
- 1UL);
+ pmbr->PartitionRecord[0].SizeInLBA =
PED_CPU_TO_LE32(dev->length - 1UL);
- return ped_device_write (dev, &pmbr, GPT_PMBR_LBA, GPT_PMBR_SECTORS);
+ int write_ok = ped_device_write (dev, pmbr, GPT_PMBR_LBA,
+ GPT_PMBR_SECTORS);
+ ped_free (pmbr);
+ return write_ok;
}
static void
@@ -1022,7 +1051,7 @@ gpt_write(const PedDisk * disk)
GPTDiskData* gpt_disk_data;
GuidPartitionEntry_t* ptes;
uint32_t ptes_crc;
- uint8_t* pth_raw = ped_malloc (pth_get_size (disk->dev));
+ uint8_t* pth_raw;
GuidPartitionTableHeader_t* gpt;
PedPartition* part;
int ptes_size;
@@ -1054,16 +1083,20 @@ gpt_write(const PedDisk * disk)
/* Write PTH and PTEs */
_generate_header (disk, 0, ptes_crc, &gpt);
pth_raw = pth_get_raw (disk->dev, gpt);
+ pth_free (gpt);
if (!ped_device_write (disk->dev, pth_raw, 1, 1))
- goto error_free_ptes;
+ goto error_free_pth_raw;
+ ped_free (pth_raw);
if (!ped_device_write (disk->dev, ptes, 2, ptes_size /
disk->dev->sector_size))
goto error_free_ptes;
/* Write Alternate PTH & PTEs */
_generate_header (disk, 1, ptes_crc, &gpt);
pth_raw = pth_get_raw (disk->dev, gpt);
+ pth_free (gpt);
if (!ped_device_write (disk->dev, pth_raw, disk->dev->length - 1, 1))
- goto error_free_ptes;
+ goto error_free_pth_raw;
+ ped_free (pth_raw);
if (!ped_device_write (disk->dev, ptes,
disk->dev->length - 1 - ptes_size /
disk->dev->sector_size,
ptes_size / disk->dev->sector_size))
@@ -1072,6 +1105,8 @@ gpt_write(const PedDisk * disk)
ped_free (ptes);
return ped_device_sync (disk->dev);
+error_free_pth_raw:
+ ped_free (pth_raw);
error_free_ptes:
ped_free (ptes);
error:
diff --git a/libparted/labels/rdb.c b/libparted/labels/rdb.c
index 483a292..c521416 100644
--- a/libparted/labels/rdb.c
+++ b/libparted/labels/rdb.c
@@ -349,7 +349,7 @@ amiga_alloc (const PedDevice* dev)
if (!(disk = _ped_disk_alloc (dev, &amiga_disk_type)))
return NULL;
- if (!(disk->disk_specific = ped_malloc (PED_SECTOR_SIZE_DEFAULT))) {
+ if (!(disk->disk_specific = ped_malloc (disk->dev->sector_size))) {
ped_free (disk);
return NULL;
}
@@ -360,7 +360,7 @@ amiga_alloc (const PedDevice* dev)
rdb->rdb_ID = PED_CPU_TO_BE32 (IDNAME_RIGIDDISK);
rdb->rdb_SummedLongs = PED_CPU_TO_BE32 (64);
rdb->rdb_HostID = PED_CPU_TO_BE32 (0);
- rdb->rdb_BlockBytes = PED_CPU_TO_BE32 (PED_SECTOR_SIZE_DEFAULT);
+ rdb->rdb_BlockBytes = PED_CPU_TO_BE32 (disk->dev->sector_size);
rdb->rdb_Flags = PED_CPU_TO_BE32 (0);
/* Block lists */
@@ -446,7 +446,7 @@ amiga_clobber (PedDevice* dev)
int result = 0;
PED_ASSERT(dev != NULL, return 0);
- if ((rdb=RDSK(ped_malloc(PED_SECTOR_SIZE_DEFAULT)))==NULL)
+ if ((rdb=RDSK(ped_malloc(dev->sector_size)))==NULL)
return 0;
while ((i = _amiga_find_rdb (dev, rdb)) != AMIGA_RDB_NOT_FOUND) {
@@ -650,14 +650,14 @@ amiga_write (const PedDisk* disk)
PED_ASSERT (disk->dev != NULL, return 0);
PED_ASSERT (disk->disk_specific != NULL, return 0);
- if (!(rdb = ped_malloc (PED_SECTOR_SIZE_DEFAULT)))
+ if (!(rdb = ped_malloc (disk->dev->sector_size)))
return 0;
/* Let's read the rdb */
if ((rdb_num = _amiga_find_rdb (disk->dev, rdb)) ==
AMIGA_RDB_NOT_FOUND) {
rdb_num = 2;
} else {
- memcpy (RDSK(disk->disk_specific), rdb,
PED_SECTOR_SIZE_DEFAULT);
+ memcpy (RDSK(disk->disk_specific), rdb, disk->dev->sector_size);
}
ped_free (rdb);
rdb = RDSK(disk->disk_specific);
@@ -676,7 +676,7 @@ amiga_write (const PedDisk* disk)
for (i = 0; i<=rdb_num; i++) table[i] = IDNAME_RIGIDDISK;
/* Let's allocate a partition block */
- if (!(block = ped_malloc (PED_SECTOR_SIZE_DEFAULT))) {
+ if (!(block = ped_malloc (disk->dev->sector_size))) {
ped_free (table);
return 0;
}
@@ -790,7 +790,7 @@ amiga_partition_new (const PedDisk* disk, PedPartitionType
part_type,
return NULL;
if (ped_partition_is_active (part)) {
- if (!(part->disk_specific = ped_malloc
(PED_SECTOR_SIZE_DEFAULT))) {
+ if (!(part->disk_specific = ped_malloc
(disk->dev->sector_size))) {
ped_free (part);
return NULL;
}
_______________________________________________
parted-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/parted-devel
