chain module: bugfixing and cosmetics

Bugs:

- one introduced ages ago - find_by_* functions should be tested vs. <0
  for failure
- one introduced by 579f15c7c456c - it accidentally blocked too much
  when testing for ',' in drive/partition option parsing
- one introduced by d4d713c9ccfe0 - sone machines / VMs seem to set
  @40:75 just 1, regardless of amount of fixed disks available

Signed-off-by: Michal Soltys <soltys@ziu.info>

diff --git a/com32/chain/chain.c b/com32/chain/chain.c
index cd2a812..b846f16 100644 (file)
--- a/com32/chain/chain.c
+++ b/com32/chain/chain.c
@@ -2,8 +2,8 @@
  *
  *   Copyright 2003-2009 H. Peter Anvin - All Rights Reserved
  *   Copyright 2009-2010 Intel Corporation; author: H. Peter Anvin
- *   Significant portions copyright (C) 2010 Shao Miller
- *                                     [partition iteration, GPT, "fs"]
+ *   Copyright 2010 Shao Miller
+ *   Copyright 2010 Michal Soltys
  *
  *   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
@@ -43,7 +43,7 @@
 
 struct options opt;
 
-static int fixed_cnt;
+static int fixed_cnt = 128;   /* see comments in main() */
 
 static int overlap(const struct data_area *a, const struct data_area *b)
 {
@@ -425,7 +425,7 @@ static int pentry_mangle(struct part_iter *_iter)
        if (opt.mbrchs) {
            wb |= pem_setchs(&iter->di, dp, (uint32_t)iter->start_lba);
            if (ridx > 4)
-               wb |= pem_setchs(&iter->di, mbr.table + 1, iter->sub.dos.ebr_lba);
+               wb |= pem_setchs(&iter->di, mbr.table + 1, iter->sub.dos.nebr_lba);
        }
     }
     /* last write */
@@ -452,14 +452,14 @@ int find_dp(struct part_iter **_iter)
     sdi = syslinux_derivative_info();
 
     if (!strncmp(opt.drivename, "mbr", 3)) {
-       if (find_by_sig(strtoul(opt.drivename + 4, NULL, 0), &iter)) {
+       if (find_by_sig(strtoul(opt.drivename + 4, NULL, 0), &iter) < 0) {
            error("Unable to find requested MBR signature.\n");
            goto bail;
        }
     } else if (!strncmp(opt.drivename, "guid", 4)) {
        if (str_to_guid(opt.drivename + 5, &gpt_guid))
            goto bail;
-       if (find_by_guid(&gpt_guid, &iter)) {
+       if (find_by_guid(&gpt_guid, &iter) < 0) {
            error("Unable to find requested GPT disk or partition by guid.\n");
            goto bail;
        }
@@ -468,7 +468,7 @@ int find_dp(struct part_iter **_iter)
            error("No label specified.\n");
            goto bail;
        }
-       if (find_by_label(opt.drivename + 6, &iter)) {
+       if (find_by_label(opt.drivename + 6, &iter) < 0) {
            error("Unable to find requested GPT partition by label.\n");
            goto bail;
        }
@@ -657,10 +657,16 @@ int main(int argc, char *argv[])
     /* Parse arguments */
     if (parse_args(argc, argv))
        goto bail;
-
+#if 0
     /* Get max fixed disk number */
     fixed_cnt = *(uint8_t *)(0x475);
 
+    /*
+     * hmm, looks like we can't do that
+     * any better options than hardcoded 0x80 - 0xFF ?
+     */
+#endif
+
     /* Get disk/part iterator matching user supplied options */
     if (find_dp(&iter))
        goto bail;
@@ -713,6 +719,7 @@ int main(int argc, char *argv[])
     } else if (opt.hand) {
        if (setup_handover(iter, &hdat))
            goto bail;
+
        /* Verify possible conflicts */
        if ( ( opt.file && overlap(&fdat, &hdat)) ||
             ( opt.sect && overlap(&sdat, &hdat) && opt.maps) ) {
@@ -723,14 +730,12 @@ int main(int argc, char *argv[])
     }
 
     /* Adjust registers */
+
     mangler_common(iter);
     mangler_handover(iter, &hdat);
     mangler_grldr(iter);
 
-    /*
-     * Patching functions
-     * opt.* are tested inside
-     */
+    /* Patching functions */
 
     if (manglef_isolinux(&fdat))
        goto bail;
@@ -757,7 +762,10 @@ int main(int argc, char *argv[])
     if (mangles_cmldr(&sdat))
        goto bail;
 
-    /* Prepare boot-time mmap data */
+    /*
+     * Prepare boot-time mmap data We should to it here, as manglers could
+     * potentially alter some of the data.
+     */
 
     if (opt.file)
        memcpy(data + ndata++, &fdat, sizeof(fdat));
@@ -771,7 +779,8 @@ int main(int argc, char *argv[])
     printf("iter idx: %d\n", iter->index);
     printf("iter lba: %llu\n", iter->start_lba);
     if (opt.hand)
-       printf("hand lba: %u\n", ((disk_dos_part_entry *)hdat.data)->start_lba);
+       printf("hand lba: %u\n",
+               ((struct disk_dos_part_entry *)hdat.data)->start_lba);
 #endif
 
     if (opt.warn) {

diff --git a/com32/chain/mangle.c b/com32/chain/mangle.c
index 34b033e..aa95738 100644 (file)
--- a/com32/chain/mangle.c
+++ b/com32/chain/mangle.c
@@ -304,8 +304,8 @@ int manglesf_bss(struct data_area *sec, struct data_area *fil)
     type1 = bpb_detect(fil->data);
     type2 = bpb_detect(sec->data);
 
-    if (type1 < 0 || type2 < 0) {
-       error("Option 'bss' can't determine BPB type.\n");
+    if (!type1 || !type2) {
+       error("Couldn't determine the BPB type for option 'bss'.\n");
        goto bail;
     }
     if (type1 != type2) {

diff --git a/com32/chain/options.c b/com32/chain/options.c
index 975224b..811776e 100644 (file)
--- a/com32/chain/options.c
+++ b/com32/chain/options.c
@@ -277,15 +277,16 @@ int parse_args(int argc, char *argv[])
                   || !strncmp(argv[i], "boot,", 5)
                   || !strcmp(argv[i], "fs")) {
            opt.drivename = argv[i];
-           if (strncmp(argv[i], "label", 5)) {
+           if (strncmp(argv[i], "label", 5))
                p = strchr(opt.drivename, ',');
-               if (p) {
-                   *p = '\0';
-                   opt.partition = p + 1;
-               } else if (argv[i + 1] && argv[i + 1][0] >= '0'
-                       && argv[i + 1][0] <= '9') {
-                   opt.partition = argv[++i];
-               }
+           else
+               p = NULL;
+           if (p) {
+               *p = '\0';
+               opt.partition = p + 1;
+           } else if (argv[i + 1] && argv[i + 1][0] >= '0'
+                   && argv[i + 1][0] <= '9') {
+               opt.partition = argv[++i];
            }
        } else {
            usage();

diff --git a/com32/chain/partiter.c b/com32/chain/partiter.c
index f76359c..ef74b68 100644 (file)
--- a/com32/chain/partiter.c
+++ b/com32/chain/partiter.c
@@ -370,7 +370,7 @@ static int prep_base_ebr(struct part_iter *iter)
        iter->sub.dos.ebr_size = iter->sub.dos.bebr_size;
 
        iter->sub.dos.cebr_lba = 0;
-       iter->sub.dos.ebr_lba = iter->sub.dos.bebr_start;
+       iter->sub.dos.nebr_lba = iter->sub.dos.bebr_start;
 
        iter->index0--;
     }
@@ -385,10 +385,10 @@ static int pi_dos_next_ebr(struct part_iter *iter, uint32_t *lba,
     if (prep_base_ebr(iter))
        return -1;
 
-    while (++iter->index0 < 1024 && iter->sub.dos.ebr_lba) {
+    while (++iter->index0 < 1024 && iter->sub.dos.nebr_lba) {
        free(iter->data);
        if (!(iter->data =
-                   disk_read_sectors(&iter->di, iter->sub.dos.ebr_lba, 1))) {
+                   disk_read_sectors(&iter->di, iter->sub.dos.nebr_lba, 1))) {
            error("Couldn't load EBR.\n");
            return -1;
        }
@@ -398,17 +398,17 @@ static int pi_dos_next_ebr(struct part_iter *iter, uint32_t *lba,
 
        dp = ((struct disk_dos_mbr *)iter->data)->table;
 
-       iter->sub.dos.cebr_lba = iter->sub.dos.ebr_lba;
+       iter->sub.dos.cebr_lba = iter->sub.dos.nebr_lba;
 
        /* setup next frame values */
        if (dp[1].ostype) {
            iter->sub.dos.ebr_start = dp[1].start_lba;
            iter->sub.dos.ebr_size = dp[1].length;
-           iter->sub.dos.ebr_lba = iter->sub.dos.bebr_start + dp[1].start_lba;
+           iter->sub.dos.nebr_lba = iter->sub.dos.bebr_start + dp[1].start_lba;
        } else {
            iter->sub.dos.ebr_start = 0;
            iter->sub.dos.ebr_size = 0;
-           iter->sub.dos.ebr_lba = 0;
+           iter->sub.dos.nebr_lba = 0;
        }
 
        if (!dp[0].ostype)

diff --git a/com32/chain/partiter.h b/com32/chain/partiter.h
index 971d388..22397bd 100644 (file)
--- a/com32/chain/partiter.h
+++ b/com32/chain/partiter.h
@@ -64,7 +64,7 @@ struct part_iter {
     union _sub {
        struct _dos {
            uint32_t disk_sig;
-           uint32_t ebr_lba;
+           uint32_t nebr_lba;
            uint32_t cebr_lba;
            /* internal */
            uint32_t ebr_start;

diff --git a/com32/chain/utility.c b/com32/chain/utility.c
index 6894305..828c85e 100644 (file)
--- a/com32/chain/utility.c
+++ b/com32/chain/utility.c
@@ -7,6 +7,19 @@
 #include <syslinux/disk.h>
 #include "utility.h"
 
+#ifdef DEBUG
+static const char *bpbtypes[] = {
+    [0] =  "BPB unknown",
+    [1] =  "BPB v.2.0",
+    [2] =  "BPB v.3.0",
+    [3] =  "BPB v.3.2",
+    [4] =  "BPB v.3.4",
+    [5] =  "BPB v.4.0",
+    [6] =  "BPB v.NT+",
+    [7] =  "BPB v.7.0",
+};
+#endif
+
 void error(const char *msg)
 {
     fputs(msg, stderr);
@@ -130,11 +143,11 @@ int drvoff_detect(int type, unsigned int *off)
  */
 int bpb_detect(const uint8_t *sec)
 {
-    int a, b, c, jmp = -1, rev = -1;
+    int a, b, c, jmp = -1, rev = 0;
 
     /* media descriptor check */
     if ((sec[0x15] & 0xF0) != 0xF0)
-       return -1;
+       goto out;
 
     if (sec[0] == 0xEB)        /* jump short */
        jmp = 2 + *(int8_t *)(sec + 1);
@@ -146,7 +159,7 @@ int bpb_detect(const uint8_t *sec)
 
     /* sanity */
     if (jmp < 0x18 || jmp > 0x1F0)
-       return -1;
+       goto out;
 
     /* detect by jump */
     if (jmp >= 0x18 && jmp < 0x1E)
@@ -160,8 +173,18 @@ int bpb_detect(const uint8_t *sec)
 
     /* TODO: some better V2 - V3.4 checks ? */
 
-    if (rev >= 0)
-       return rev;
+    if (rev)
+       goto out;
+    /*
+     * BPB info:
+     * 2.0 ==       0x0B - 0x17
+     * 3.0 == 2.0 + 0x18 - 0x1D
+     * 3.2 == 3.0 + 0x1E - 0x1F
+     * 3.4 ==!2.0 + 0x18 - 0x23
+     * 4.0 == 3.4 + 0x24 - 0x45
+     *  NT ==~3.4 + 0x24 - 0x53
+     * 7.0 == 3.4 + 0x24 - 0x59
+     */
 
 nocode:
     a = memcmp(sec + 0x03, "NTFS", 4);
@@ -176,6 +199,10 @@ nocode:
        rev = bpbV70;
     }
 
+out:
+#ifdef DEBUG
+    printf("INFO: BPB detection: %s\n", bpbtypes[rev]);
+#endif
     return rev;
 }
 

diff --git a/com32/chain/utility.h b/com32/chain/utility.h
index 0cdb36f..4f50e0e 100644 (file)
--- a/com32/chain/utility.h
+++ b/com32/chain/utility.h
@@ -4,6 +4,7 @@
 #include <stdint.h>
 #include <syslinux/disk.h>
 
+#define bpbUNK 0
 #define bpbV20 1
 #define bpbV30 2
 #define bpbV32 3

diff --git a/doc/chain.txt b/doc/chain.txt
index 1443c24..886625d 100644 (file)
--- a/doc/chain.txt
+++ b/doc/chain.txt
@@ -263,8 +263,7 @@ This is used for loading of *only* Dell's DOS derivatives. It does require boot
 sector at 0x2000 and overall valid BPB values. As in other DOS-ish cases,
 likely candidates for use are 'save' and 'hide'.
 
-       grub=<file>
-       grubcfg=<config>
+       grub=<file> [grubcfg=<config>]
        sets: file=<file> seg=0x800::0x200 nohand nosect grub
 
 Chainloads grub legacy's stage2, performing additional corrections on the file
@@ -282,13 +281,13 @@ in memory.
 
 This emulates syslinux's native BSS option. This loads both the file and the
 sector, adjusts BPB values in the loaded sector, then copies all possible BPB
-fields to the loaded file. Everything is made with reference to selected
+fields to the loaded file. Everything is made with reference to the selected
 disk/partition.
 
        bs=<file>
        sets: bs=<file> nosect filebpb
 
 This emulates syslinux's native BS option. This loads the file and if possible
-- adjusts its BPB values. Everything is made with reference to selected
+- adjusts its BPB values. Everything is made with reference to the selected
 disk/partition.