[PATCH] normalize default prefix in grub-mkimage

[PATCH] normalize default prefix in grub-mkimage

[Andrey Borzenkov]

This fixes bogus "C:\boot\grub" at runtime for Windows build as well as
the case when /boot is on separate mount point for other systems.

---
 util/grub-mkimage.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
index a2bd4c1..4d54f0a 100644
--- a/util/grub-mkimage.c
+++ b/util/grub-mkimage.c
@@ -84,6 +84,7 @@ static struct argp_option options[] = {
   { 0, 0, 0, 0, 0, 0 }
 };
 
+static char *default_prefix;
 #pragma GCC diagnostic ignored "-Wformat-nonliteral"
 
 static char *
@@ -94,7 +95,7 @@ help_filter (int key, const char *text, void *input __attribute__ ((unused)))
     case 'd':
       return xasprintf (text, grub_util_get_pkglibdir ());
     case 'p':
-      return xasprintf (text, DEFAULT_DIRECTORY);
+      return xasprintf (text, default_prefix);
     case 'O':
       {
 	char *formats = grub_install_get_image_targets_string (), *ret;
@@ -245,6 +246,7 @@ main (int argc, char *argv[])
 
   grub_util_host_init (&argc, &argv);
 
+  default_prefix = grub_make_system_path_relative_to_its_root (DEFAULT_DIRECTORY);
   memset (&arguments, 0, sizeof (struct arguments));
   arguments.comp = GRUB_COMPRESSION_AUTO;
   arguments.modules_max = argc + 1;
@@ -252,6 +254,7 @@ main (int argc, char *argv[])
 			     * sizeof (arguments.modules[0]));
   memset (arguments.modules, 0, (arguments.modules_max + 1)
 	  * sizeof (arguments.modules[0]));
+  arguments.prefix = xstrdup (default_prefix);
 
   if (argp_parse (&argp, argc, argv, 0, 0, &arguments) != 0)
     {
@@ -287,8 +290,7 @@ main (int argc, char *argv[])
       strcpy (ptr, dn);
     }
 
-  grub_install_generate_image (arguments.dir,
-			       arguments.prefix ? : DEFAULT_DIRECTORY, fp,
+  grub_install_generate_image (arguments.dir, arguments.prefix, fp,
 			       arguments.output, arguments.modules,
 			       arguments.memdisk, arguments.pubkeys,
 			       arguments.npubkeys, arguments.config,
@@ -298,6 +300,8 @@ main (int argc, char *argv[])
   grub_util_file_sync  (fp);
   fclose (fp);
 
+  free (default_prefix);
+
   if (arguments.dir)
     free (arguments.dir);
 
-- 
tg: (645ff63..) u/mingw/default-prefix (depends on: master)

Re: [PATCH] normalize default prefix in grub-mkimage

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 27.12.2013 18:41, Andrey Borzenkov wrote:
> This fixes bogus "C:\boot\grub" at runtime for Windows build as well as
> the case when /boot is on separate mount point for other systems.
> 
grub-mkimage shouldn't perform any FS-based resolving, that's job of the
caller. This default should either be hardwired to
"/"GRUB_BOOT_DIR_NAME"/"GRUB_DIR_NAME
or perhaps even to "/boot/grub" or default should be killed altogether.
> ---
>  util/grub-mkimage.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
> index a2bd4c1..4d54f0a 100644
> --- a/util/grub-mkimage.c
> +++ b/util/grub-mkimage.c
> @@ -84,6 +84,7 @@ static struct argp_option options[] = {
>    { 0, 0, 0, 0, 0, 0 }
>  };
>  
> +static char *default_prefix;
>  #pragma GCC diagnostic ignored "-Wformat-nonliteral"
>  
>  static char *
> @@ -94,7 +95,7 @@ help_filter (int key, const char *text, void *input __attribute__ ((unused)))
>      case 'd':
>        return xasprintf (text, grub_util_get_pkglibdir ());
>      case 'p':
> -      return xasprintf (text, DEFAULT_DIRECTORY);
> +      return xasprintf (text, default_prefix);
>      case 'O':
>        {
>  	char *formats = grub_install_get_image_targets_string (), *ret;
> @@ -245,6 +246,7 @@ main (int argc, char *argv[])
>  
>    grub_util_host_init (&argc, &argv);
>  
> +  default_prefix = grub_make_system_path_relative_to_its_root (DEFAULT_DIRECTORY);
>    memset (&arguments, 0, sizeof (struct arguments));
>    arguments.comp = GRUB_COMPRESSION_AUTO;
>    arguments.modules_max = argc + 1;
> @@ -252,6 +254,7 @@ main (int argc, char *argv[])
>  			     * sizeof (arguments.modules[0]));
>    memset (arguments.modules, 0, (arguments.modules_max + 1)
>  	  * sizeof (arguments.modules[0]));
> +  arguments.prefix = xstrdup (default_prefix);
>  
>    if (argp_parse (&argp, argc, argv, 0, 0, &arguments) != 0)
>      {
> @@ -287,8 +290,7 @@ main (int argc, char *argv[])
>        strcpy (ptr, dn);
>      }
>  
> -  grub_install_generate_image (arguments.dir,
> -			       arguments.prefix ? : DEFAULT_DIRECTORY, fp,
> +  grub_install_generate_image (arguments.dir, arguments.prefix, fp,
>  			       arguments.output, arguments.modules,
>  			       arguments.memdisk, arguments.pubkeys,
>  			       arguments.npubkeys, arguments.config,
> @@ -298,6 +300,8 @@ main (int argc, char *argv[])
>    grub_util_file_sync  (fp);
>    fclose (fp);
>  
> +  free (default_prefix);
> +
>    if (arguments.dir)
>      free (arguments.dir);
>  
> 



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

Re: [PATCH] normalize default prefix in grub-mkimage

[Andrey Borzenkov]

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

В Tue, 21 Jan 2014 08:19:24 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> On 27.12.2013 18:41, Andrey Borzenkov wrote:
> > This fixes bogus "C:\boot\grub" at runtime for Windows build as well as
> > the case when /boot is on separate mount point for other systems.
> > 
> grub-mkimage shouldn't perform any FS-based resolving, that's job of the
> caller. This default should either be hardwired to
> "/"GRUB_BOOT_DIR_NAME"/"GRUB_DIR_NAME
> or perhaps even to "/boot/grub" or default should be killed altogether.

I prefer the latter. Any default will most likely be wrong, so it is
better to be explicit about it.

From: Andrey Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] do not set default prefix in grub-mkimage

Default prefix is likely wrong on Unix and completely wrong on Windows.
Let caller set it explicitly to avoid any ambiguity.

---
 util/grub-mkimage.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
index a2bd4c1..1e0bcf1 100644
--- a/util/grub-mkimage.c
+++ b/util/grub-mkimage.c
@@ -64,7 +64,7 @@ static struct argp_option options[] = {
   {"directory",  'd', N_("DIR"), 0,
    /* TRANSLATORS: platform here isn't identifier. It can be translated.  */
    N_("use images and modules under DIR [default=%s/<platform>]"), 0},
-  {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory [default=%s]"), 0},
+  {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory"), 0},
   {"memdisk",  'm', N_("FILE"), 0,
    /* TRANSLATORS: "memdisk" here isn't an identifier, it can be translated.
     "embed" is a verb (command description).  "*/
@@ -93,8 +93,6 @@ help_filter (int key, const char *text, void *input __attribute__ ((unused)))
     {
     case 'd':
       return xasprintf (text, grub_util_get_pkglibdir ());
-    case 'p':
-      return xasprintf (text, DEFAULT_DIRECTORY);
     case 'O':
       {
 	char *formats = grub_install_get_image_targets_string (), *ret;
@@ -268,6 +266,15 @@ main (int argc, char *argv[])
       exit(1);
     }
 
+  if (!arguments.prefix)
+    {
+      char *program = xstrdup(program_name);
+      printf ("%s\n", _("Prefix not specified (use the -p option)."));
+      argp_help (&argp, stderr, ARGP_HELP_STD_USAGE, program);
+      free (program);
+      exit(1);
+    }
+
   if (arguments.output)
     {
       fp = grub_util_fopen (arguments.output, "wb");
@@ -287,8 +294,7 @@ main (int argc, char *argv[])
       strcpy (ptr, dn);
     }
 
-  grub_install_generate_image (arguments.dir,
-			       arguments.prefix ? : DEFAULT_DIRECTORY, fp,
+  grub_install_generate_image (arguments.dir, arguments.prefix, fp,
 			       arguments.output, arguments.modules,
 			       arguments.memdisk, arguments.pubkeys,
 			       arguments.npubkeys, arguments.config,
-- 
tg: (443b322..) u/mingw/default-prefix (depends on: master)

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

Re: [PATCH] normalize default prefix in grub-mkimage

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 21.01.2014 17:48, Andrey Borzenkov wrote:
> В Tue, 21 Jan 2014 08:19:24 +0100
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
>> On 27.12.2013 18:41, Andrey Borzenkov wrote:
>>> This fixes bogus "C:\boot\grub" at runtime for Windows build as well as
>>> the case when /boot is on separate mount point for other systems.
>>>
>> grub-mkimage shouldn't perform any FS-based resolving, that's job of the
>> caller. This default should either be hardwired to
>> "/"GRUB_BOOT_DIR_NAME"/"GRUB_DIR_NAME
>> or perhaps even to "/boot/grub" or default should be killed altogether.
> 
> I prefer the latter. Any default will most likely be wrong, so it is
> better to be explicit about it.
> 
If you call grub-mkimage directly you're either dev or doing sth wrong.
The default allows quicker creation of test images for new systems where
you don't expect it to get to normal mode yet anyway. This being said, I
agree with restraining possibilities of direct-tool-usage screwup.
Go ahead.
> From: Andrey Borzenkov <arvidjaar@gmail.com>
> Subject: [PATCH] do not set default prefix in grub-mkimage
> 
> Default prefix is likely wrong on Unix and completely wrong on Windows.
> Let caller set it explicitly to avoid any ambiguity.
> 
> ---
>  util/grub-mkimage.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
> index a2bd4c1..1e0bcf1 100644
> --- a/util/grub-mkimage.c
> +++ b/util/grub-mkimage.c
> @@ -64,7 +64,7 @@ static struct argp_option options[] = {
>    {"directory",  'd', N_("DIR"), 0,
>     /* TRANSLATORS: platform here isn't identifier. It can be translated.  */
>     N_("use images and modules under DIR [default=%s/<platform>]"), 0},
> -  {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory [default=%s]"), 0},
> +  {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory"), 0},
>    {"memdisk",  'm', N_("FILE"), 0,
>     /* TRANSLATORS: "memdisk" here isn't an identifier, it can be translated.
>      "embed" is a verb (command description).  "*/
> @@ -93,8 +93,6 @@ help_filter (int key, const char *text, void *input __attribute__ ((unused)))
>      {
>      case 'd':
>        return xasprintf (text, grub_util_get_pkglibdir ());
> -    case 'p':
> -      return xasprintf (text, DEFAULT_DIRECTORY);
>      case 'O':
>        {
>  	char *formats = grub_install_get_image_targets_string (), *ret;
> @@ -268,6 +266,15 @@ main (int argc, char *argv[])
>        exit(1);
>      }
>  
> +  if (!arguments.prefix)
> +    {
> +      char *program = xstrdup(program_name);
> +      printf ("%s\n", _("Prefix not specified (use the -p option)."));
> +      argp_help (&argp, stderr, ARGP_HELP_STD_USAGE, program);
> +      free (program);
> +      exit(1);
> +    }
> +
>    if (arguments.output)
>      {
>        fp = grub_util_fopen (arguments.output, "wb");
> @@ -287,8 +294,7 @@ main (int argc, char *argv[])
>        strcpy (ptr, dn);
>      }
>  
> -  grub_install_generate_image (arguments.dir,
> -			       arguments.prefix ? : DEFAULT_DIRECTORY, fp,
> +  grub_install_generate_image (arguments.dir, arguments.prefix, fp,
>  			       arguments.output, arguments.modules,
>  			       arguments.memdisk, arguments.pubkeys,
>  			       arguments.npubkeys, arguments.config,
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



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