[parted-devel] [PATCH parted] gpt: Fix endianness issue when using pth->FirstUsableLBA.

[Jim Meyering]

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 <parted-owner at fedoraproject.org>
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 <meyering at redhat.com>
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