Richard W.M. Jones wrote: > Full explanation is here: > > https://bugzilla.redhat.com/show_bug.cgi?id=829960#c6
Hi Rich, Thanks a lot for the fix. I've amended your commit to add a little more of a log and to include a NEWS update mentioning the bug fix. Then I added a test to give us coverage of the affected function, since no other test actually used that code. Since I've amended your commit, I'll wait for an ACK from you before pushing it. >From 21f5333fd903fd31680ec2c90693041b22a7c66a Mon Sep 17 00:00:00 2001 From: Fedora Ninjas <[email protected]> Date: Fri, 8 Jun 2012 13:19:25 +0100 Subject: [PATCH 1/2] gpt: fix endianness bug in gpt_get_max_supported_partition_count * libparted/labels/gpt.c (gpt_get_max_supported_partition_count): Take endianness of pth->FirstUsableLBA into account (64-bit, little endian) when calculating the maximum number of partitions. * NEWS (Bug fixes): Mention it. --- NEWS | 3 +++ libparted/labels/gpt.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 3969c44..f929b99 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,9 @@ GNU parted NEWS -*- outline -*- ** Bug Fixes + libparted: gpt: fix gpt_get_max_supported_partition_count to work + also on little-endian systems. + libparted: treat a disk with no pMBR as an msdos-labeled disk even when it has valid GPT headers. diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c index 91ad71a..6032e3f 100644 --- a/libparted/labels/gpt.c +++ b/libparted/labels/gpt.c @@ -1785,12 +1785,12 @@ gpt_get_max_supported_partition_count (const PedDisk *disk, int *max_n) if (!_header_is_valid (disk, pth, 1)) { - pth->FirstUsableLBA = 34; + pth->FirstUsableLBA = PED_CPU_TO_LE64 (34); pth->SizeOfPartitionEntry = PED_CPU_TO_LE32 (sizeof (GuidPartitionEntry_t)); } - *max_n = (disk->dev->sector_size * (pth->FirstUsableLBA - 2) + *max_n = (disk->dev->sector_size * (PED_LE64_TO_CPU (pth->FirstUsableLBA) - 2) / PED_LE32_TO_CPU (pth->SizeOfPartitionEntry)); pth_free (pth); return true; -- 1.7.11.rc2 >From 6d851a0b80c7bc8341ca3ea1bf05d2f60a65b797 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[email protected]> Date: Sat, 9 Jun 2012 17:26:21 +0200 Subject: [PATCH 2/2] tests: add a test to exercise just-fixed code * tests/print-max.c: Extend to provide coverage of ped_disk_get_max_supported_partition_count, too. * tests/t9021-maxima.sh (max_n_partitions): New function, with naively hard-coded max-number-of-partitions-per-partition-table-type values. Use it to ensure that each expected value matches the actual one. * cfg.mk: Exempt this test's use of error from the syntax-check for unmarked diagnostics. --- cfg.mk | 2 ++ tests/print-max.c | 10 ++++++++++ tests/t9021-maxima.sh | 26 +++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/cfg.mk b/cfg.mk index c6a00c8..45d2ac2 100644 --- a/cfg.mk +++ b/cfg.mk @@ -69,3 +69,5 @@ exclude_file_name_regexp--sc_prohibit_always-defined_macros = \ exclude_file_name_regexp--sc_prohibit_path_max_allocation = \ ^libparted/arch/beos\.c$$ + +exclude_file_name_regexp--sc_unmarked_diagnostics = ^tests/print-max\.c$$ diff --git a/tests/print-max.c b/tests/print-max.c index 7560d49..41aa8c6 100644 --- a/tests/print-max.c +++ b/tests/print-max.c @@ -2,9 +2,11 @@ #include <parted/parted.h> #include <stdio.h> #include <stdlib.h> +#include <errno.h> #include "closeout.h" #include "progname.h" +#include "error.h" int main (int argc, char **argv) @@ -26,8 +28,16 @@ main (int argc, char **argv) PedSector max_length = ped_disk_max_partition_length (disk); PedSector max_start_sector = ped_disk_max_partition_start_sector (disk); + if (!ped_device_open(dev)) + error (EXIT_FAILURE, errno, "failed to open %s\n", dev_name); + int max_n_partitions; + bool ok = ped_disk_get_max_supported_partition_count (disk, + &max_n_partitions); + printf ("max len: %llu\n", (unsigned long long) max_length); printf ("max start sector: %llu\n", (unsigned long long) max_start_sector); + printf ("max number of partitions: %d\n", + ok ? max_n_partitions : -1); ped_disk_destroy (disk); ped_device_destroy (dev); diff --git a/tests/t9021-maxima.sh b/tests/t9021-maxima.sh index 0570585..ca10d17 100755 --- a/tests/t9021-maxima.sh +++ b/tests/t9021-maxima.sh @@ -23,6 +23,27 @@ dev=dev-file PATH="..:$PATH" export PATH +max_n_partitions() +{ + case $1 in + + # Technically, msdos partition tables have no limit on the maximum number + # of partitions, but we pretend it is 64 due to implementation details. + msdos) m=64;; + + gpt) m=128;; + dvh) m=16;; + sun) m=8;; + mac) m=65536;; + bsd) m=8;; + amiga) m=128;; + loop) m=1;; + pc98) case $ss in 512) m=16;; *) m=64;; esac;; + *) warn_ invalid partition table type: $1 1>&2; exit 1;; + esac + echo $m +} + # FIXME: add aix when/if it's supported again for t in msdos gpt dvh sun mac bsd amiga loop pc98; do echo $t @@ -40,8 +61,11 @@ for t in msdos gpt dvh sun mac bsd amiga loop pc98; do esac print-max $dev > out 2>&1 || fail=1 + m=$(max_n_partitions $t) || fail=1 printf '%s\n' "max len: $max_len" \ - "max start sector: $max_start" > exp || fail=1 + "max start sector: $max_start" \ + "max number of partitions: $m" \ + > exp || fail=1 compare exp out || fail=1 done -- 1.7.11.rc2
