I'm going to push these today.
FWIW, they were prompted by still more changes
that I'm working on, that might help us get much
better test coverage for s390x/dasd.
These are mostly clean-up and test-adding changes,
but do include two small bug fixes:
ped_disk_type_get_next: fix a const-correctness bug
tests: rewrite t0202 to use new framework
tests: weaken t0202-gpt-pmbr to assume a preexisting GPT table
dvh: replace open-coded dvh_clobber with equivalent, shorter code
FIX gpt_probe: don't attempt to read beyond end of a very small disk
FIX linux_read: give a proper diagnostic for an "end of file" error
tests: t0000-basic.sh: minor correction
gpt: greatly simply gpt_clobber; minor semantic change, too
>From 666cd7fca6ec2a5db364bd4de47dd64c1c373905 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Fri, 13 Nov 2009 09:44:30 +0100
Subject: [PATCH 1/8] ped_disk_type_get_next: fix a const-correctness bug
* libparted/disk.c (ped_disk_type_get_next): Make param const.
* include/parted/disk.h (ped_disk_type_get_next): Update prototype.
---
include/parted/disk.h | 3 +--
libparted/disk.c | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/parted/disk.h b/include/parted/disk.h
index 76397a8..434f0d3 100644
--- a/include/parted/disk.h
+++ b/include/parted/disk.h
@@ -239,7 +239,7 @@ struct _PedDiskArchOps {
extern void ped_disk_type_register (PedDiskType* type);
extern void ped_disk_type_unregister (PedDiskType* type);
-extern PedDiskType* ped_disk_type_get_next (PedDiskType* type);
+extern PedDiskType* ped_disk_type_get_next (PedDiskType const *type);
extern PedDiskType* ped_disk_type_get (const char* name);
extern int ped_disk_type_check_feature (const PedDiskType* disk_type,
PedDiskTypeFeature feature);
@@ -354,4 +354,3 @@ extern int _ped_partition_attempt_align (
#endif /* PED_DISK_H_INCLUDED */
/** @} */
-
diff --git a/libparted/disk.c b/libparted/disk.c
index f29140e..8c4b229 100644
--- a/libparted/disk.c
+++ b/libparted/disk.c
@@ -101,7 +101,7 @@ ped_disk_type_unregister (PedDiskType* disk_type)
* \return Next disk; NULL if "type" is the last registered disk type.
*/
PedDiskType*
-ped_disk_type_get_next (PedDiskType* type)
+ped_disk_type_get_next (PedDiskType const *type)
{
if (type)
return type->next;
--
1.6.5.2.372.gc0502
>From 180cfdf56f1d65ae1564d0a8f750417c09738ad5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Fri, 13 Nov 2009 09:45:49 +0100
Subject: [PATCH 2/8] tests: rewrite t0202 to use new framework
* tests/t0202-gpt-pmbr.sh: Rewrite.
---
tests/t0202-gpt-pmbr.sh | 45 +++++++++++++++++++++++----------------------
1 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/tests/t0202-gpt-pmbr.sh b/tests/t0202-gpt-pmbr.sh
index 996cfe9..4ca520b 100755
--- a/tests/t0202-gpt-pmbr.sh
+++ b/tests/t0202-gpt-pmbr.sh
@@ -1,4 +1,5 @@
#!/bin/sh
+# Preserve first 446B of the Protected MBR for gpt partitions.
# Copyright (C) 2009 Free Software Foundation, Inc.
@@ -15,36 +16,36 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-test_description='Preserve first 446B of the Protected MBR for gpt partitions.'
+if test "$VERBOSE" = yes; then
+ set -x
+ parted --version
+fi
: ${srcdir=.}
-. $srcdir/test-lib.sh
+. $srcdir/t-lib.sh
dev=loop-file
bootcode_size=446
-test_expect_success \
- 'Create a 100k test file with random content' \
- 'printf %0${bootcode_size}d 0 > in &&
- dd if=in of=$dev bs=1c count=$bootcode_size &&
- dd if=/dev/zero of=$dev bs=1c seek=$bootcode_size \
- count=101954 > /dev/null 2>&1'
+fail=0
+# Create a 100k test file with random content
+printf %0${bootcode_size}d 0 > in || fail=1
+dd if=in of=$dev bs=1c count=$bootcode_size || fail=1
+dd if=/dev/zero of=$dev bs=1c seek=$bootcode_size \
+ count=101954 > /dev/null 2>&1 || fail=1
-test_expect_success \
- 'Extract the first $bootcode_size Bytes before GPT creation' \
- 'dd if=$dev of=before bs=1c count=$bootcode_size > /dev/null 2>&1'
+# Extract the first $bootcode_size Bytes before GPT creation
+dd if=$dev of=before bs=1c count=$bootcode_size > /dev/null 2>&1 || fail=1
-test_expect_success \
- 'create a GPT partition table' \
- 'parted -s $dev mklabel gpt > out 2>&1'
-test_expect_success 'expect no output' 'compare out /dev/null'
+# create a GPT partition table
+parted -s $dev mklabel gpt > out 2>&1 || fail=1
+# expect no output
+compare out /dev/null
-test_expect_success \
- 'Extract the first $bootcode_size Bytes after GPT creation' \
- 'dd if=$dev of=after bs=1c count=$bootcode_size > /dev/null 2>&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
-test_expect_success \
- 'Compare the before and after' \
- 'compare before after'
+# Compare the before and after
+compare before after || fail=1
-test_done
+Exit $fail
--
1.6.5.2.372.gc0502
>From 2233f093b828c1e2f340cd377c1aee6b40424076 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Fri, 13 Nov 2009 11:38:34 +0100
Subject: [PATCH 3/8] tests: weaken t0202-gpt-pmbr to assume a preexisting GPT
table
* tests/t0202-gpt-pmbr.sh: Lay down an initial GPT table before
writing to the MBR, so this test passes with the new semantics.
---
tests/t0202-gpt-pmbr.sh | 21 +++++++++++++--------
1 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/tests/t0202-gpt-pmbr.sh b/tests/t0202-gpt-pmbr.sh
index 4ca520b..209dcc7 100755
--- a/tests/t0202-gpt-pmbr.sh
+++ b/tests/t0202-gpt-pmbr.sh
@@ -28,16 +28,21 @@ dev=loop-file
bootcode_size=446
fail=0
-# Create a 100k test file with random content
+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
+
+# Fill the first $bootcode_size bytes with 0's.
+# This affects only the protective MBR, so doesn't affect validity of gpt
table.
printf %0${bootcode_size}d 0 > in || fail=1
-dd if=in of=$dev bs=1c count=$bootcode_size || fail=1
-dd if=/dev/zero of=$dev bs=1c seek=$bootcode_size \
- count=101954 > /dev/null 2>&1 || fail=1
+dd of=$dev bs=1 seek=0 count=$bootcode_size conv=notrunc < in || fail=1
-# Extract the first $bootcode_size Bytes before GPT creation
-dd if=$dev of=before bs=1c count=$bootcode_size > /dev/null 2>&1 || fail=1
+parted -s $dev p || fail=1
-# create a GPT partition table
+# 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
@@ -46,6 +51,6 @@ compare out /dev/null
dd if=$dev of=after bs=1c count=$bootcode_size > /dev/null 2>&1 || fail=1
# Compare the before and after
-compare before after || fail=1
+compare in after || fail=1
Exit $fail
--
1.6.5.2.372.gc0502
>From 9348e4fa3e61cec94fa6c986d4fb5d6f83e7903c Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Fri, 13 Nov 2009 11:39:40 +0100
Subject: [PATCH 4/8] dvh: replace open-coded dvh_clobber with equivalent,
shorter code
* libparted/labels/dvh.c (dvh_clobber): Simply use ptt_clear_sectors.
---
libparted/labels/dvh.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/libparted/labels/dvh.c b/libparted/labels/dvh.c
index 2f7db3d..9049b86 100644
--- a/libparted/labels/dvh.c
+++ b/libparted/labels/dvh.c
@@ -98,12 +98,7 @@ dvh_probe (const PedDevice *dev)
static int
dvh_clobber (PedDevice* dev)
{
- char *zeros = ped_calloc (dev->sector_size);
- if (zeros == NULL)
- return 0;
- int ok = ped_device_write (dev, zeros, 0, 1);
- free (zeros);
- return ok;
+ return ptt_clear_sectors (dev, 0, 1);
}
#endif /* !DISCOVER_ONLY */
--
1.6.5.2.372.gc0502
>From 4312698a11abed988b5bacca0ecbec151de045e3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Fri, 13 Nov 2009 13:40:29 +0100
Subject: [PATCH 5/8] gpt_probe: don't attempt to read beyond end of a very
small disk
* libparted/labels/gpt.c (gpt_probe): Don't try to read the
2nd sector if that's beyond the end of the disk.
* tests/t0001-tiny.sh: New test, to expose the above boundary-case bug.
Part of the msdos-partition-creation process involves probing for
other types of partition tables. Probing for gpt would evoke a
nonsensical diagnostic.
* tests/Makefile.am (TESTS): Add t0001-tiny.sh.
---
libparted/labels/gpt.c | 3 ++
tests/Makefile.am | 1 +
tests/t0001-tiny.sh | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 55 insertions(+), 0 deletions(-)
create mode 100755 tests/t0001-tiny.sh
diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
index 5037d9b..aca555a 100644
--- a/libparted/labels/gpt.c
+++ b/libparted/labels/gpt.c
@@ -444,6 +444,9 @@ gpt_probe (const PedDevice *dev)
PED_ASSERT (dev != NULL, return 0);
+ if (dev->length <= 1)
+ return 0;
+
if (ped_device_read (dev, pth_raw, 1, GPT_HEADER_SECTORS)
|| ped_device_read (dev, pth_raw, dev->length - 1, GPT_HEADER_SECTORS))
{
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 79c01b3..ece3f64 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,5 +1,6 @@
TESTS = \
t0000-basic.sh \
+ t0001-tiny.sh \
t0010-script-no-ctrl-chars.sh \
t0100-print.sh \
t0200-gpt.sh \
diff --git a/tests/t0001-tiny.sh b/tests/t0001-tiny.sh
new file mode 100755
index 0000000..373a68d
--- /dev/null
+++ b/tests/t0001-tiny.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+# operate on a very small (1-sector) "disk"
+
+# Copyright (C) 2009 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
+
+ss=$sector_size_
+dev=loop-file
+
+fail=0
+
+for opt in '' -s; do
+
+ dd if=/dev/null of=$dev bs=1 seek=$ss || framework_failure
+
+ # create an msdos partition table:
+ # Before parted-2.1, without -s, this would fail with a bogus diagnostic:
+ # Error: Success during read on .../tests/loop-file
+ # Retry/Ignore/Cancel? ^C
+ parted $opt $dev mklabel msdos ---pretend-input-tty </dev/null > out 2>&1 \
+ || fail=1
+ # expect no output
+ sed 's/.*WARNING: You are not superuser.*//;/^$/d' out > k && mv k out ||
fail=1
+ compare out /dev/null || fail=1
+
+ parted -s $dev p || fail=1
+ rm -f $dev
+
+done
+
+Exit $fail
--
1.6.5.2.372.gc0502
>From 6a84782b40b888ed839512afbc0f75d97911799d Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Fri, 13 Nov 2009 13:43:58 +0100
Subject: [PATCH 6/8] linux_read: give a proper diagnostic for an "end of file"
error
Before today's gpt_probe bug fix, its test case would provoke
an invalid diagnostic. This makes the diagnostic useful.
* libparted/arch/linux.c (linux_read): When hitting EOF while reading,
diagnose it properly, rather than via strerror(0) (i.e., "Success").
---
libparted/arch/linux.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 744f854..4fe48ec 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -1711,7 +1711,9 @@ linux_read (const PedDevice* dev, void* buffer, PedSector
start,
ex_status = ped_exception_throw (
PED_EXCEPTION_ERROR,
PED_EXCEPTION_RETRY_IGNORE_CANCEL,
- _("%s during read on %s"),
+ (status == 0
+ ? _("end of file while reading %s")
+ : _("%s during read on %s")),
strerror (errno),
dev->path);
--
1.6.5.2.372.gc0502
>From 2f3323d502ed0649b9e66c24e9ed11160b7ad0c3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Fri, 13 Nov 2009 13:47:29 +0100
Subject: [PATCH 7/8] tests: t0000-basic.sh: minor correction
* tests/t0000-basic.sh: When zeroing out the first sector,
use dd's conv=notrunc. Otherwise, we'd also truncate the
backing file size to 4KiB.
---
tests/t0000-basic.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tests/t0000-basic.sh b/tests/t0000-basic.sh
index 1e0f7ca..43103d2 100755
--- a/tests/t0000-basic.sh
+++ b/tests/t0000-basic.sh
@@ -49,7 +49,7 @@ test_expect_success 'expect no output' 'compare out /dev/null'
test_expect_success \
'erase the left-over label' \
- 'dd if=/dev/zero of=$dev bs=4K count=1 2> /dev/null'
+ 'dd if=/dev/zero of=$dev bs=4K count=1 conv=notrunc 2> /dev/null'
# First iteration works with no prompting, since there is no preexisting label.
test_expect_success \
--
1.6.5.2.372.gc0502
>From 71bea9d928ea871abc81da0165dabfa997cd31c5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Fri, 13 Nov 2009 13:51:53 +0100
Subject: [PATCH 8/8] gpt: greatly simply gpt_clobber; minor semantic change, too
* libparted/labels/gpt.c (gpt_clobber): Don't bother to read and
parse existing headers. Instead, simply clear three sectors:
the pMBR, the primary header (LBA1), and the last sector.
There is no point in clearing what the primary header says is the
AlternateLBA, since once the primary header and the last sector
are cleared, there is no risk of any tool using it by mistake.
---
libparted/labels/gpt.c | 47 +++++------------------------------------------
1 files changed, 5 insertions(+), 42 deletions(-)
diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
index aca555a..e6347e3 100644
--- a/libparted/labels/gpt.c
+++ b/libparted/labels/gpt.c
@@ -490,53 +490,16 @@ gpt_probe (const PedDevice *dev)
}
#ifndef DISCOVER_ONLY
-/* writes zeros to the PMBR and the primary and alternate GPTHs and PTEs */
+/* writes zeros to the PMBR and the primary GPTH, and to the final sector */
static int
gpt_clobber (PedDevice *dev)
{
- uint8_t *pth_raw = ped_malloc (pth_get_size (dev));
- GuidPartitionTableHeader_t *gpt;
-
PED_ASSERT (dev != NULL, return 0);
- /*
- * TO DISCUSS: check whether checksum is correct?
- * If not, we might get a wrong AlternateLBA field and destroy
- * one sector of random data.
- */
- if (!ped_device_read (dev, pth_raw,
- GPT_PRIMARY_HEADER_LBA, GPT_HEADER_SECTORS))
- {
- free (pth_raw);
- return 0;
- }
-
- gpt = pth_new_from_raw (dev, pth_raw);
- free (pth_raw);
-
- if (!ptt_clear_sectors (dev, GPT_PMBR_LBA, GPT_PMBR_SECTORS))
- goto error_free_with_gpt;
- if (!ptt_clear_sectors (dev, GPT_PRIMARY_HEADER_LBA, GPT_HEADER_SECTORS))
- goto error_free_with_gpt;
- if (!ptt_clear_sectors (dev, dev->length - GPT_HEADER_SECTORS,
- GPT_HEADER_SECTORS))
- goto error_free_with_gpt;
-
- if ((PedSector) PED_LE64_TO_CPU (gpt->AlternateLBA) < dev->length - 1)
- {
- if (!ped_device_write (dev, gpt,
- PED_LE64_TO_CPU (gpt->AlternateLBA),
- GPT_HEADER_SECTORS))
- return 0;
- }
-
- pth_free (gpt);
-
- return 1;
-
-error_free_with_gpt:
- pth_free (gpt);
- return 0;
+ return (ptt_clear_sectors (dev, GPT_PMBR_LBA, GPT_PMBR_SECTORS)
+ && ptt_clear_sectors (dev, GPT_PRIMARY_HEADER_LBA,
GPT_HEADER_SECTORS)
+ && ptt_clear_sectors (dev, dev->length - GPT_HEADER_SECTORS,
+ GPT_HEADER_SECTORS));
}
#endif /* !DISCOVER_ONLY */
--
1.6.5.2.372.gc0502
_______________________________________________
parted-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/parted-devel
