Brian C. Lane wrote: > From: "Brian C. Lane" <b...@redhat.com> > > 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 > --- > libparted/labels/pc98.c | 32 ++++++++++---------------------- > 1 files changed, 10 insertions(+), 22 deletions(-) > > diff --git a/libparted/labels/pc98.c b/libparted/labels/pc98.c > index 3afa8a2..ea3cf4e 100644 > --- a/libparted/labels/pc98.c > +++ b/libparted/labels/pc98.c > @@ -140,7 +140,14 @@ 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); > + 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; > } > > static int > @@ -192,27 +199,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) > + /* check for boot loader signatures */ > + if (!pc98_check_ipl_signature (&part_table)) > return 0; > > return 1;
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. 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); Finally, please always configure with ./configure --enable-gcc-warnings That would have highlighted some unused variables and the unused function. _______________________________________________ bug-parted mailing list bug-parted@gnu.org https://lists.gnu.org/mailman/listinfo/bug-parted