Re: Wrong partition identifier for HFS

[Jim Meyering]

Curtis Gedak wrote:
> When an "hfs" partition is created on a msdos disk, the partition
> identifier is set to "0x83".  The correct value for an HFS partition
> is "0xAF".
>
> The identifier "0xAF" is used by Apple for MacOS X file systems HFS or
> HFS+ on Intel.
>
> Following is a link to a list of partition identifiers, a set of
> commands to demonstrate the problem, and a proposed patch to fix this
> problem.
...
> Subject: [PATCH] dos: set hfs partition identifier to proper value
>
> The correct value for an "hfs" partition identifier is 0xAF.
> Previously the partition identifier was incorrectly set to 0x83.
> ---
> libparted/labels/dos.c |    5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/libparted/labels/dos.c b/libparted/labels/dos.c
> index dce35b2..69c177c 100644
> --- a/libparted/labels/dos.c
> +++ b/libparted/labels/dos.c
> @@ -86,6 +86,7 @@ static const char MBR_BOOT_CODE[] = {
> #define PARTITION_LINUX        0x83
> #define PARTITION_LINUX_EXT    0x85
> #define PARTITION_LINUX_LVM    0x8e
> +#define PARTITION_HFS        0xaf
> #define PARTITION_SUN_UFS    0xbf
> #define PARTITION_DELL_DIAG    0xde
> #define PARTITION_GPT        0xee

Thank you!
Just in time.
I was about to release 2.2.

I've adjusted your patch to work also with hfs+, wrote the NEWS
entry and added a test to exercise it.
(BTW, your mail client mangled the patch, so I couldn't apply it)

Along the way I noticed two cases in which unwanted
output on stdout would fail to provoke a test failure.
That's the first patch:

>From 15b05d910328f4120f5ded3b81f5f29e861f8b31 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 26 Feb 2010 14:47:52 +0100
Subject: [PATCH 1/3] tests: gpt-pmbr: don't ignore stdout comparison result

* tests/t0202-gpt-pmbr.sh: Fail if we get unexpected output.
---
 tests/t0202-gpt-pmbr.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/t0202-gpt-pmbr.sh b/tests/t0202-gpt-pmbr.sh
index acfb2b6..100194a 100755
--- a/tests/t0202-gpt-pmbr.sh
+++ b/tests/t0202-gpt-pmbr.sh
@@ -33,7 +33,7 @@ dd if=/dev/null of=$dev bs=1 seek=1M || framework_failure
 # create a GPT partition table
 parted -s $dev mklabel gpt > out 2>&1 || fail=1
 # expect no output
-compare out /dev/null
+compare out /dev/null || fail=1

 # Fill the first $bootcode_size bytes with 0's.
 # This affects only the protective MBR, so doesn't affect validity of gpt 
table.
@@ -45,7 +45,7 @@ parted -s $dev p || fail=1
 # create a GPT partition table on top of the existing one.
 parted -s $dev mklabel gpt > out 2>&1 || fail=1
 # expect no output
-compare out /dev/null
+compare out /dev/null || fail=1

 # Extract the first $bootcode_size Bytes after GPT creation
 dd if=$dev of=after bs=1c count=$bootcode_size > /dev/null 2>&1 || fail=1
--
1.7.0.442.g55ad1


>From 4da2a2c02c0a86224d0465f10fd24aa625c285e6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 26 Feb 2010 14:53:33 +0100
Subject: [PATCH 2/3] tests: exercise today's HFS partition type fix

* tests/t2400-dos-hfs-partition-type.sh: New script.
* tests/Makefile.am: Add it.
* NEWS (Bug fixes): Mention it.
---
 NEWS                                  |    4 +++
 tests/Makefile.am                     |    1 +
 tests/t2400-dos-hfs-partition-type.sh |   48 +++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 0 deletions(-)
 create mode 100644 tests/t2400-dos-hfs-partition-type.sh

diff --git a/NEWS b/NEWS
index 53f83ce..c390fcd 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,10 @@ GNU parted NEWS                                    -*- 
outline -*-
   Parted no longer uses a physical sector size of 0 or of any other
   value smaller than the logical sector size.

+  dos: creating an HFS or HFS+ partition in an msdos partition table
+  used to set the partition type to 0x83.  That is wrong.  The required
+  number is 0xaf, and that is what is used now.
+
   gpt: read-only operation could clobber MBR part of hybrid GPT+MBR table
   [bug introduced in parted-2.1]

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8159a32..0cc7279 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -22,6 +22,7 @@ TESTS = \
   t2100-mkswap.sh \
   t2200-dos-label-recog.sh \
   t2300-dos-label-extended-bootcode.sh \
+  t2400-dos-hfs-partition-type.sh \
   t3000-resize-fs.sh \
   t4000-sun-raid-type.sh \
   t4001-sun-vtoc.sh \
diff --git a/tests/t2400-dos-hfs-partition-type.sh 
b/tests/t2400-dos-hfs-partition-type.sh
new file mode 100644
index 0000000..1b1ffe6
--- /dev/null
+++ b/tests/t2400-dos-hfs-partition-type.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+# Ensure that an HFS partition in a dos table gets the right ID
+
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  parted --version
+fi
+
+: ${srcdir=.}
+. $srcdir/t-lib.sh
+
+dev=loop-file
+ss=$sector_size_
+n_sectors=8000
+
+fail=0
+
+dd if=/dev/null of=$dev bs=$ss seek=$n_sectors || framework_failure
+
+# create a GPT partition table
+parted -s $dev mklabel msdos \
+  mkpart pri hfs  2048s 4095s \
+  mkpart pri hfs+ 4096s 6143s > out 2>&1 || fail=1
+# expect no output
+compare out /dev/null || fail=1
+
+# Extract the "type" byte of the first partition.
+od -An -j450 -tx1 -N1 $dev  > out || fail=1
+od -An -j466 -tx1 -N1 $dev >> out || fail=1
+printf ' af\n af\n' > exp || fail=1
+compare out exp || fail=1
+
+Exit $fail
--
1.7.0.442.g55ad1


>From 9ffd7eb93f6027a35ff854f7518947c91c086136 Mon Sep 17 00:00:00 2001
From: Curtis  Gedak <address@hidden>
Date: Fri, 26 Feb 2010 15:07:29 +0100
Subject: [PATCH 3/3] dos: set HFS/HFS+ partition identifier to proper value

The correct value for an "hfs" partition identifier is 0xAF.
Previously the partition identifier was incorrectly set to 0x83.
* libparted/labels/dos.c (PARTITION_HFS): Define.
(msdos_partition_set_system): Use it.
---
 libparted/labels/dos.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/libparted/labels/dos.c b/libparted/labels/dos.c
index dce35b2..b83bcc2 100644
--- a/libparted/labels/dos.c
+++ b/libparted/labels/dos.c
@@ -86,6 +86,7 @@ static const char MBR_BOOT_CODE[] = {
 #define PARTITION_LINUX                0x83
 #define PARTITION_LINUX_EXT    0x85
 #define PARTITION_LINUX_LVM    0x8e
+#define PARTITION_HFS          0xaf
 #define PARTITION_SUN_UFS      0xbf
 #define PARTITION_DELL_DIAG    0xde
 #define PARTITION_GPT          0xee
@@ -1354,7 +1355,10 @@ msdos_partition_set_system (PedPartition* part,
                   || !strcmp (fs_type->name, "hpfs")) {
                dos_data->system = PARTITION_NTFS;
                dos_data->system |= dos_data->hidden ? PART_FLAG_HIDDEN : 0;
-       } else if (!strcmp (fs_type->name, "sun-ufs"))
+       } else if (!strcmp (fs_type->name, "hfs")
+                  || !strcmp (fs_type->name, "hfs+"))
+               dos_data->system = PARTITION_HFS;
+       else if (!strcmp (fs_type->name, "sun-ufs"))
                dos_data->system = PARTITION_SUN_UFS;
        else if (is_linux_swap (fs_type->name))
                dos_data->system = PARTITION_LINUX_SWAP;
--
1.7.0.442.g55ad1