[PATCH v2 0/2] util/grub-install-common: Print usable grub-mkimage command

[PATCH v2 0/2] util/grub-install-common: Print usable grub-mkimage command

[Glenn Washburn]

This fixes an issue where grub-install does not print a grub-mkimage
command, when running in verbose more, that can be run by grub-mkimage.
Specifically, --dtb and --sbat will not take empty strings as arguments.

changes from v1:
 - Split into two patches to make changes more clear, no other code changes

Glenn

Glenn Washburn (2):
  util/grub-install-common: Minor improvements to printing of
    grub-mkimage command
  util/grub-install-common: Print usable grub-mkimage command

 util/grub-install-common.c | 49 ++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 18 deletions(-)

-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 1/2] util/grub-install-common: Minor improvements to printing of grub-mkimage command

[Glenn Washburn]

This is a prepratory patch to make the following patch less cluttered. The
only visible change made here is to not print extra spaces when either or
both --note or --disable-shim-lock are not given and to not print an extra
space at the end of the command. The latter is done by constructing the
trailing argument string with spaces in front of each argument rather than
trailing. The allocation of the argument string is made precise, which has
the benefit of saving a few bytes, but more importantly self-documenting
what the needed allocated bytes are. Also, unneeded braces are removed from
an if block.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 util/grub-install-common.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index 52a29d1cb8e0..f9b9201c894e 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -617,60 +617,58 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
   int dc = decompressors ();
 
   if (memdisk_path)
-    slen += 20 + grub_strlen (memdisk_path);
+    slen += sizeof (" --memdisk ''") + grub_strlen (memdisk_path);
   if (config_path)
-    slen += 20 + grub_strlen (config_path);
+    slen += sizeof (" --config ''") + grub_strlen (config_path);
 
   for (pk = pubkeys; pk < pubkeys + npubkeys; pk++)
-    slen += 20 + grub_strlen (*pk);
+    slen += sizeof (" --pubkey ''") + grub_strlen (*pk);
 
   for (md = modules.entries; *md; md++)
-    {
-      slen += 10 + grub_strlen (*md);
-    }
+    slen += sizeof (" ''") + grub_strlen (*md);
 
   p = s = xmalloc (slen);
   if (memdisk_path)
     {
+      *p++ = ' ';
       p = grub_stpcpy (p, "--memdisk '");
       p = grub_stpcpy (p, memdisk_path);
       *p++ = '\'';
-      *p++ = ' ';
     }
   if (config_path)
     {
+      *p++ = ' ';
       p = grub_stpcpy (p, "--config '");
       p = grub_stpcpy (p, config_path);
       *p++ = '\'';
-      *p++ = ' ';
     }
   for (pk = pubkeys; pk < pubkeys + npubkeys; pk++)
     {
+      *p++ = ' ';
       p = grub_stpcpy (p, "--pubkey '");
       p = grub_stpcpy (p, *pk);
       *p++ = '\'';
-      *p++ = ' ';
     }
 
   for (md = modules.entries; *md; md++)
     {
+      *p++ = ' ';
       *p++ = '\'';
       p = grub_stpcpy (p, *md);
       *p++ = '\'';
-      *p++ = ' ';
     }
 
   *p = '\0';
 
-  grub_util_info ("grub-mkimage --directory '%s' --prefix '%s'"
-		  " --output '%s' "
+  grub_util_info ("grub-mkimage --directory '%s' --prefix '%s' --output '%s'"
 		  " --dtb '%s' "
 		  "--sbat '%s' "
-		  "--format '%s' --compression '%s' %s %s %s\n",
-		  dir, prefix,
-		  outname, dtb ? : "", sbat ? : "", mkimage_target,
-		  compnames[compression], note ? "--note" : "",
-		  disable_shim_lock ? "--disable-shim-lock" : "", s);
+		  "--format '%s' --compression '%s'%s%s%s\n",
+		  dir, prefix, outname,
+		  dtb ? : "", sbat ? : "",
+		  mkimage_target, compnames[compression],
+		  note ? " --note" : "",
+		  disable_shim_lock ? " --disable-shim-lock" : "", s);
   free (s);
 
   tgt = grub_install_get_image_target (mkimage_target);
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 2/2] util/grub-install-common: Print usable grub-mkimage command

[Glenn Washburn]

When grub-install is run with the verbose option, it will print a log
message indicating the grub-mkimage command and arguments used.
GRUB no longer calls the grub-mkimage binary internally, however the
command logged is a command that if run should effectively be what
grub-install used. However, as this has changed some of the newer
options have been incorrectly added so that the printed command fails
when run separately. This change makes the displayed command run as
intended.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 util/grub-install-common.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index f9b9201c894e..014741945e15 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -620,6 +620,10 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
     slen += sizeof (" --memdisk ''") + grub_strlen (memdisk_path);
   if (config_path)
     slen += sizeof (" --config ''") + grub_strlen (config_path);
+  if (dtb)
+    slen += sizeof (" --dtb ''") + grub_strlen (dtb);
+  if (sbat)
+    slen += sizeof (" --sbat ''") + grub_strlen (sbat);
 
   for (pk = pubkeys; pk < pubkeys + npubkeys; pk++)
     slen += sizeof (" --pubkey ''") + grub_strlen (*pk);
@@ -642,6 +646,20 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
       p = grub_stpcpy (p, config_path);
       *p++ = '\'';
     }
+  if (dtb)
+    {
+      *p++ = ' ';
+      p = grub_stpcpy (p, "--dtb '");
+      p = grub_stpcpy (p, dtb);
+      *p++ = '\'';
+    }
+  if (sbat)
+    {
+      *p++ = ' ';
+      p = grub_stpcpy (p, "--sbat '");
+      p = grub_stpcpy (p, dtb);
+      *p++ = '\'';
+    }
   for (pk = pubkeys; pk < pubkeys + npubkeys; pk++)
     {
       *p++ = ' ';
@@ -661,11 +679,8 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
   *p = '\0';
 
   grub_util_info ("grub-mkimage --directory '%s' --prefix '%s' --output '%s'"
-		  " --dtb '%s' "
-		  "--sbat '%s' "
-		  "--format '%s' --compression '%s'%s%s%s\n",
+		  " --format '%s' --compression '%s'%s%s%s\n",
 		  dir, prefix, outname,
-		  dtb ? : "", sbat ? : "",
 		  mkimage_target, compnames[compression],
 		  note ? " --note" : "",
 		  disable_shim_lock ? " --disable-shim-lock" : "", s);
-- 
2.34.1


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 0/2] util/grub-install-common: Print usable grub-mkimage command

[Daniel Kiper]

On Fri, Sep 22, 2023 at 02:34:16PM -0500, Glenn Washburn wrote:
> This fixes an issue where grub-install does not print a grub-mkimage
> command, when running in verbose more, that can be run by grub-mkimage.
> Specifically, --dtb and --sbat will not take empty strings as arguments.
>
> changes from v1:
>  - Split into two patches to make changes more clear, no other code changes
>
> Glenn
>
> Glenn Washburn (2):
>   util/grub-install-common: Minor improvements to printing of
>     grub-mkimage command
>   util/grub-install-common: Print usable grub-mkimage command

For both patches Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel