On Sat, Oct 15, 2011 at 11:10:38AM +0200, Jim Meyering wrote: > Hi Brian, > > Thanks for the patch. > However, do you really intend to remove those check_partition_consistency > calls? That seems like it would weaken the test unnecessarily.
As far as I can tell there isn't a consensus on what is valid for PC98 so I opted to just make it signature based. > > Also, would you please change the name to pc98_valid_ipl_signature? > That will make it more readable for me. With the "_check_" name, > I find that I have to read the code to determine the semantics. > While with "_valid_...", it's obviously a boolean. > And then you can change the last four lines to be simply this: > > return pc98_valid_ipl_signature (&part_table); > Actually, _check_ is consistent with the code (eg. the check_magic code). > Finally, please always configure with > > ./configure --enable-gcc-warnings > > That would have highlighted some unused variables and the unused function. Sorry about that, I build it using the fedora build scripts and missed that one. Enclosed is an updated patch. -- Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
From 942e75eb8e605be49aa86cf3b35462ce807772de Mon Sep 17 00:00:00 2001
From: "Brian C. Lane" <b...@redhat.com>
Date: Fri, 7 Oct 2011 16:53:49 -0700
Subject: [PATCH] libparted: make pc98 detection depend on signatures
(#646053)
pc98 is not a common disk label. Change pc98_probe to only return true
if one of the recognized signatures is present.
Currently these include:
IPL1
Linux 98
GRUB/98
This will prevent false-positive detection on msdos labeled disks
* libparted/labels/pc98.c (pc98_probe): Change to require signature
(pc98_check_ipl_signature): Add more signatures
(check_partition_consistency): Remove unused function
---
libparted/labels/pc98.c | 65 ++++++----------------------------------------
1 files changed, 9 insertions(+), 56 deletions(-)
diff --git a/libparted/labels/pc98.c b/libparted/labels/pc98.c
index 3afa8a2..d60843d 100644
--- a/libparted/labels/pc98.c
+++ b/libparted/labels/pc98.c
@@ -140,45 +140,20 @@ pc98_check_magic (const PC98RawTable *part_table)
static int
pc98_check_ipl_signature (const PC98RawTable *part_table)
{
- return !memcmp (part_table->boot_code + 4, "IPL1", 4);
-}
-
-static int
-check_partition_consistency (const PedDevice* dev,
- const PC98RawPartition* raw_part)
-{
- if (raw_part->ipl_sect >= dev->hw_geom.sectors
- || raw_part->sector >= dev->hw_geom.sectors
- || raw_part->end_sector >= dev->hw_geom.sectors
- || raw_part->ipl_head >= dev->hw_geom.heads
- || raw_part->head >= dev->hw_geom.heads
- || raw_part->end_head >= dev->hw_geom.heads
- || PED_LE16_TO_CPU(raw_part->ipl_cyl) >= dev->hw_geom.cylinders
- || PED_LE16_TO_CPU(raw_part->cyl) >= dev->hw_geom.cylinders
- || PED_LE16_TO_CPU(raw_part->end_cyl) >= dev->hw_geom.cylinders
- || PED_LE16_TO_CPU(raw_part->cyl)
- > PED_LE16_TO_CPU(raw_part->end_cyl)
-#if 0
- || !chs_to_sector(dev, PED_LE16_TO_CPU(raw_part->ipl_cyl),
- raw_part->ipl_head, raw_part->ipl_sect)
- || !chs_to_sector(dev, PED_LE16_TO_CPU(raw_part->cyl),
- raw_part->head, raw_part->sector)
- || !chs_to_sector(dev, PED_LE16_TO_CPU(raw_part->end_cyl),
- raw_part->end_head, raw_part->end_sector)
-#endif
- || PED_LE16_TO_CPU(raw_part->end_cyl)
- < PED_LE16_TO_CPU(raw_part->cyl))
+ if (memcmp (part_table->boot_code + 4, "IPL1", 4) == 0)
+ return 1;
+ else if (memcmp (part_table->boot_code + 4, "Linux 98", 8) == 0)
+ return 1;
+ else if (memcmp (part_table->boot_code + 4, "GRUB/98 ", 8) == 0)
+ return 1;
+ else
return 0;
-
- return 1;
}
static int
pc98_probe (const PedDevice *dev)
{
PC98RawTable part_table;
- int empty;
- const PC98RawPartition* p;
PED_ASSERT (dev != NULL);
@@ -192,30 +167,8 @@ pc98_probe (const PedDevice *dev)
if (!pc98_check_magic (&part_table))
return 0;
- /* check consistency */
- empty = 1;
- for (p = part_table.partitions;
- p < part_table.partitions + MAX_PART_COUNT;
- p++)
- {
- if (p->mid == 0 && p->sid == 0)
- continue;
- empty = 0;
- if (!check_partition_consistency (dev, p))
- return 0;
- }
-
- /* check boot loader */
- if (pc98_check_ipl_signature (&part_table))
- return 1;
- else if (part_table.boot_code[0]) /* invalid boot loader */
- return 0;
-
- /* Not to mistake msdos disk map for PC-9800's empty disk map */
- if (empty)
- return 0;
-
- return 1;
+ /* check for boot loader signatures */
+ return pc98_check_ipl_signature (&part_table);
}
static PedDisk*
--
1.7.6.4
pgp3agCSMgzeU.pgp
Description: PGP signature
_______________________________________________ bug-parted mailing list bug-parted@gnu.org https://lists.gnu.org/mailman/listinfo/bug-parted
