i386/pc/chainloader.c fails with syntax like chainloader (hd1, msdos1)+1

* i386/pc/chainloader.c fails with syntax like chainloader (hd1, msdos1)+1
@ 2013-06-21  9:23 Michael Chang
  2013-06-21 11:48 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Chang @ 2013-06-21  9:23 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 920 bytes --]

Hi,

The i386/pc/chainloader.c hangs when attempting to chainload other
disk's boot sector (or mbr) with a syntax like this. Say if current
environment setting root=hd0,msdos2.

  $ chainloader (hd1,msdos1)+1

However doing it this way always works.

  $ set root=hd1,msdos1
  $ chainloader +1

I suppose the first syntax is valid as it should override $root [1]
thus two cases should have identical result. But looking into the
chainloader source it seems that the $root is always used to obtain
the boot drive number and partition table. So there's a discrepancy if
root is not set to the chainloader device which is explicitly
specified in file.

The attached file fixes the problem for me, I'd like to contribute if
you think this is a problem or how to improve the patch.

[1] http://www.gnu.org/software/grub/manual/grub.html#root
[2] http://www.gnu.org/software/grub/manual/grub.html#chainloader

Thanks,
Michael

[-- Attachment #2: chainloader-drive-number.patch --]
[-- Type: text/x-patch, Size: 1887 bytes --]

Index: grub-2.00/grub-core/loader/i386/pc/chainloader.c
===================================================================

--- grub-2.00.orig/grub-core/loader/i386/pc/chainloader.c
+++ grub-2.00/grub-core/loader/i386/pc/chainloader.c
@@ -39,6 +39,7 @@
 #include <grub/fat.h>
 #include <grub/ntfs.h>
 #include <grub/i386/relocator.h>
+#include <grub/env.h>
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
@@ -149,6 +150,7 @@ grub_chainloader_cmd (const char *filena
   int drive = -1;
   grub_addr_t part_addr = 0;
   grub_uint8_t *bs, *ptable;
+  const char *biosnum;
 
   rel = grub_relocator_new ();
   if (!rel)
@@ -198,11 +200,28 @@ grub_chainloader_cmd (const char *filena
       goto fail;
     }
 
-  grub_file_close (file);
 
-  /* Obtain the partition table from the root device.  */
-  drive = grub_get_root_biosnumber ();
-  dev = grub_device_open (0);
+  /* Obtain the partition table from the loaded device.  */
+  dev = file->device;
+  biosnum = grub_env_get ("biosnum");
+
+  if (biosnum)
+    {
+      drive = grub_strtoul (biosnum, 0, 0);
+    }
+  else
+    {
+      if (dev->disk && dev->disk->dev
+          && dev->disk->dev->id == GRUB_DISK_DEVICE_BIOSDISK_ID)
+        drive = (int) dev->disk->id;
+    }
+
+  if (drive == -1)
+    {
+      grub_error (GRUB_ERR_BAD_NUMBER, "invalid biosnum");
+      goto fail;
+    }
+
   if (dev && dev->disk && dev->disk->partition)
     {
       grub_disk_t disk = dev->disk;
@@ -225,15 +244,14 @@ grub_chainloader_cmd (const char *filena
   if (flags & GRUB_CHAINLOADER_BPB)
     grub_chainloader_patch_bpb ((void *) 0x7C00, dev, drive);
 
-  if (dev)
-    grub_device_close (dev);
- 
   /* Ignore errors. Perhaps it's not fatal.  */
   grub_errno = GRUB_ERR_NONE;
 
   boot_drive = drive;
   boot_part_addr = part_addr;
 
+  grub_file_close (file);
+
   grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 1);
   return;
 

^ permalink raw reply	[flat|nested] 3+ messages in thread