[PATCH] fix using grub device name as install device

[PATCH] fix using grub device name as install device

[Andrey Borzenkov]

Shell version of grub-install called grub-setup which resolved
install device name and called main setup routine. C version of
grub-install calls main setup routine directly, which leads
to the error:

grub2-install: info: grub-bios-setup  --verbose  --force  --skip-fs-probe --directory='/boot/grub2/i386-pc' --device-map='/boot/grub2/device.map' '(hd2)'.
grub2-install: info: reading /boot/grub2/i386-pc/boot.img.
grub2-install: info: reading /boot/grub2/i386-pc/core.img.
grub2-install: info: root is `(null)', dest is `(hd2)'.
grub2-install: info: Opening dest.
grub2-install: info: drive = -1.
grub2-install: error: disk `(hd2)' not found.

Move resolving of destination device name into main setup routine
so it is done consistently in both cases.

---
 util/grub-setup.c | 41 +----------------------------------------
 util/setup.c      | 26 +++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/util/grub-setup.c b/util/grub-setup.c
index 90b9de0..cc3af5d 100644
--- a/util/grub-setup.c
+++ b/util/grub-setup.c
@@ -209,23 +209,9 @@ DEVICE must be an OS device (e.g. /dev/sda)."),
   NULL, help_filter, NULL
 };
 
-static char *
-get_device_name (char *dev)
-{
-  size_t len = strlen (dev);
-
-  if (dev[0] != '(' || dev[len - 1] != ')')
-    return 0;
-
-  dev[len - 1] = '\0';
-  return dev + 1;
-}
-
 int
 main (int argc, char *argv[])
 {
-  char *root_dev = NULL;
-  char *dest_dev = NULL;
   struct arguments arguments;
 
   grub_util_host_init (&argc, &argv);
@@ -264,34 +250,11 @@ main (int argc, char *argv[])
   grub_mdraid1x_init ();
   grub_lvm_init ();
 
-  dest_dev = get_device_name (arguments.device);
-  if (! dest_dev)
-    {
-      /* Possibly, the user specified an OS device file.  */
-      dest_dev = grub_util_get_grub_dev (arguments.device);
-      if (! dest_dev)
-        {
-          char *program = xstrdup(program_name);
-          fprintf (stderr, _("Invalid device `%s'.\n"), arguments.device);
-          argp_help (&argp, stderr, ARGP_HELP_STD_USAGE, program);
-          free(program);
-          exit(1);
-        }
-      grub_util_info ("transformed OS device `%s' into GRUB device `%s'",
-                      arguments.device, dest_dev);
-    }
-  else
-    {
-      /* For simplicity.  */
-      dest_dev = xstrdup (dest_dev);
-      grub_util_info ("Using `%s' as GRUB device", dest_dev);
-    }
-
   /* Do the real work.  */
   GRUB_SETUP_FUNC (arguments.dir ? : DEFAULT_DIRECTORY,
 		   arguments.boot_file ? : DEFAULT_BOOT_FILE,
 		   arguments.core_file ? : DEFAULT_CORE_FILE,
-		   dest_dev, arguments.force,
+		   arguments.device, arguments.force,
 		   arguments.fs_probe, arguments.allow_floppy);
 
   /* Free resources.  */
@@ -303,8 +266,6 @@ main (int argc, char *argv[])
   free (arguments.dir);
   free (arguments.dev_map);
   free (arguments.device);
-  free (root_dev);
-  free (dest_dev);
 
   return 0;
 }
diff --git a/util/setup.c b/util/setup.c
index 337c304..c1de3d2 100644
--- a/util/setup.c
+++ b/util/setup.c
@@ -247,12 +247,13 @@ identify_partmap (grub_disk_t disk __attribute__ ((unused)),
 void
 SETUP (const char *dir,
        const char *boot_file, const char *core_file,
-       const char *dest, int force,
+       const char *dev, int force,
        int fs_probe, int allow_floppy)
 {
   char *core_path;
   char *boot_img, *core_img, *boot_path;
   char *root = 0;
+  char *dest = 0;
   size_t boot_size, core_size;
 #ifdef GRUB_SETUP_BIOS
   grub_uint16_t core_sectors;
@@ -269,6 +270,28 @@ SETUP (const char *dir,
 #endif
   bl.last_length = 0;
 
+  {
+    size_t len = strlen (dev);
+
+    if (len > 2 && dev[0] == '(' && dev[len - 1] == ')')
+      {
+	dest = xmalloc (len - 1);
+	strncpy (dest, dev + 1, len - 2);
+	dest[len - 2] = '\0';
+      }
+  }
+
+  if (! dest)
+    {
+      /* Possibly, the user specified an OS device file.  */
+      dest = grub_util_get_grub_dev (dev);
+      if (! dest)
+          grub_util_error (_("Invalid device `%s'.\n"), dev);
+      grub_util_info ("transformed OS device `%s' into GRUB device `%s'",
+                      dev, dest);
+    }
+
+
   /* Read the boot image by the OS service.  */
   boot_path = grub_util_get_path (dir, boot_file);
   boot_size = grub_util_get_image_size (boot_path);
@@ -303,6 +326,7 @@ SETUP (const char *dir,
   dest_dev = grub_device_open (dest);
   if (! dest_dev)
     grub_util_error ("%s", grub_errmsg);
+  free (dest);
 
   core_dev = dest_dev;
 
-- 
tg: (b67422d..) u/grub-setup-device (depends on: master)

Re: [PATCH] fix using grub device name as install device

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 28.11.2013 22:11, Andrey Borzenkov wrote:
> Shell version of grub-install called grub-setup which resolved
> install device name and called main setup routine. C version of
> grub-install calls main setup routine directly, which leads
> to the error:
> 
Do you have a reason for continuing accepting such input? It was
considered wrong way of doing things for quite some while.
> grub2-install: info: grub-bios-setup  --verbose  --force  --skip-fs-probe --directory='/boot/grub2/i386-pc' --device-map='/boot/grub2/device.map' '(hd2)'.
> grub2-install: info: reading /boot/grub2/i386-pc/boot.img.
> grub2-install: info: reading /boot/grub2/i386-pc/core.img.
> grub2-install: info: root is `(null)', dest is `(hd2)'.
> grub2-install: info: Opening dest.
> grub2-install: info: drive = -1.
> grub2-install: error: disk `(hd2)' not found.
> 
> Move resolving of destination device name into main setup routine
> so it is done consistently in both cases.
> 
> ---
>  util/grub-setup.c | 41 +----------------------------------------
>  util/setup.c      | 26 +++++++++++++++++++++++++-
>  2 files changed, 26 insertions(+), 41 deletions(-)
> 
> diff --git a/util/grub-setup.c b/util/grub-setup.c
> index 90b9de0..cc3af5d 100644
> --- a/util/grub-setup.c
> +++ b/util/grub-setup.c
> @@ -209,23 +209,9 @@ DEVICE must be an OS device (e.g. /dev/sda)."),
>    NULL, help_filter, NULL
>  };
>  
> -static char *
> -get_device_name (char *dev)
> -{
> -  size_t len = strlen (dev);
> -
> -  if (dev[0] != '(' || dev[len - 1] != ')')
> -    return 0;
> -
> -  dev[len - 1] = '\0';
> -  return dev + 1;
> -}
> -
>  int
>  main (int argc, char *argv[])
>  {
> -  char *root_dev = NULL;
> -  char *dest_dev = NULL;
>    struct arguments arguments;
>  
>    grub_util_host_init (&argc, &argv);
> @@ -264,34 +250,11 @@ main (int argc, char *argv[])
>    grub_mdraid1x_init ();
>    grub_lvm_init ();
>  
> -  dest_dev = get_device_name (arguments.device);
> -  if (! dest_dev)
> -    {
> -      /* Possibly, the user specified an OS device file.  */
> -      dest_dev = grub_util_get_grub_dev (arguments.device);
> -      if (! dest_dev)
> -        {
> -          char *program = xstrdup(program_name);
> -          fprintf (stderr, _("Invalid device `%s'.\n"), arguments.device);
> -          argp_help (&argp, stderr, ARGP_HELP_STD_USAGE, program);
> -          free(program);
> -          exit(1);
> -        }
> -      grub_util_info ("transformed OS device `%s' into GRUB device `%s'",
> -                      arguments.device, dest_dev);
> -    }
> -  else
> -    {
> -      /* For simplicity.  */
> -      dest_dev = xstrdup (dest_dev);
> -      grub_util_info ("Using `%s' as GRUB device", dest_dev);
> -    }
> -
>    /* Do the real work.  */
>    GRUB_SETUP_FUNC (arguments.dir ? : DEFAULT_DIRECTORY,
>  		   arguments.boot_file ? : DEFAULT_BOOT_FILE,
>  		   arguments.core_file ? : DEFAULT_CORE_FILE,
> -		   dest_dev, arguments.force,
> +		   arguments.device, arguments.force,
>  		   arguments.fs_probe, arguments.allow_floppy);
>  
>    /* Free resources.  */
> @@ -303,8 +266,6 @@ main (int argc, char *argv[])
>    free (arguments.dir);
>    free (arguments.dev_map);
>    free (arguments.device);
> -  free (root_dev);
> -  free (dest_dev);
>  
>    return 0;
>  }
> diff --git a/util/setup.c b/util/setup.c
> index 337c304..c1de3d2 100644
> --- a/util/setup.c
> +++ b/util/setup.c
> @@ -247,12 +247,13 @@ identify_partmap (grub_disk_t disk __attribute__ ((unused)),
>  void
>  SETUP (const char *dir,
>         const char *boot_file, const char *core_file,
> -       const char *dest, int force,
> +       const char *dev, int force,
>         int fs_probe, int allow_floppy)
>  {
>    char *core_path;
>    char *boot_img, *core_img, *boot_path;
>    char *root = 0;
> +  char *dest = 0;
>    size_t boot_size, core_size;
>  #ifdef GRUB_SETUP_BIOS
>    grub_uint16_t core_sectors;
> @@ -269,6 +270,28 @@ SETUP (const char *dir,
>  #endif
>    bl.last_length = 0;
>  
> +  {
> +    size_t len = strlen (dev);
> +
> +    if (len > 2 && dev[0] == '(' && dev[len - 1] == ')')
> +      {
> +	dest = xmalloc (len - 1);
> +	strncpy (dest, dev + 1, len - 2);
> +	dest[len - 2] = '\0';
> +      }
> +  }
> +
> +  if (! dest)
> +    {
> +      /* Possibly, the user specified an OS device file.  */
> +      dest = grub_util_get_grub_dev (dev);
> +      if (! dest)
> +          grub_util_error (_("Invalid device `%s'.\n"), dev);
> +      grub_util_info ("transformed OS device `%s' into GRUB device `%s'",
> +                      dev, dest);
> +    }
> +
> +
>    /* Read the boot image by the OS service.  */
>    boot_path = grub_util_get_path (dir, boot_file);
>    boot_size = grub_util_get_image_size (boot_path);
> @@ -303,6 +326,7 @@ SETUP (const char *dir,
>    dest_dev = grub_device_open (dest);
>    if (! dest_dev)
>      grub_util_error ("%s", grub_errmsg);
> +  free (dest);
>  
>    core_dev = dest_dev;
>  
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]

Re: [PATCH] fix using grub device name as install device

[Andrey Borzenkov]

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

В Fri, 29 Nov 2013 07:27:06 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> On 28.11.2013 22:11, Andrey Borzenkov wrote:
> > Shell version of grub-install called grub-setup which resolved
> > install device name and called main setup routine. C version of
> > grub-install calls main setup routine directly, which leads
> > to the error:
> > 
> Do you have a reason for continuing accepting such input? It was
> considered wrong way of doing things for quite some while.

Yes - compatibility. At least openSUSE used this syntax during
bootloader configuration (together with maintaining device.map) in past
releases, so anyone updating will inherit it. How do you think I hit it
in the first place? :) And I do not know how many other distros are
affected.

Patch does not add any new feature - it is plain regression fix. 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] fix using grub device name as install device

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 29.11.2013 07:52, Andrey Borzenkov wrote:
> В Fri, 29 Nov 2013 07:27:06 +0100
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
>> On 28.11.2013 22:11, Andrey Borzenkov wrote:
>>> Shell version of grub-install called grub-setup which resolved
>>> install device name and called main setup routine. C version of
>>> grub-install calls main setup routine directly, which leads
>>> to the error:
>>>
>> Do you have a reason for continuing accepting such input? It was
>> considered wrong way of doing things for quite some while.
> 
> Yes - compatibility. At least openSUSE used this syntax during
> bootloader configuration (together with maintaining device.map) in past
> releases, so anyone updating will inherit it. How do you think I hit it
> in the first place? :) And I do not know how many other distros are
> affected.
> 
You're right. And as much as I hate this syntax, we're stuck with it. Go
ahead


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]