[XEN,2/3] read a grubenv file if it is next to the grub.cfg file

Commit Message

Michael Young Oct. 25, 2019, 10:52 p.m. UTC

When a grub.cfg file is found this patch checks if there is grubenv
file in the same directory as the grub.cfg file. If there is it
passes the contents to parse().

Signed-off-by: Michael Young <[e-mail]>
---
 tools/pygrub/src/pygrub | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Comments

Ian Jackson Oct. 28, 2019, 4:41 p.m. UTC | #1

YOUNG, MICHAEL A. writes ("[XEN PATCH 2/3] read a grubenv file if it is next to the grub.cfg file"):
> When a grub.cfg file is found this patch checks if there is grubenv
> file in the same directory as the grub.cfg file. If there is it
> passes the contents to parse().

I am happy with the semantics of this patch.  But I am not really
happy with the duplication of the code to call self.cf.parse, so that
it is now quadriplicated.

> +        if fenv != "" and fs.file_exists(fenv):
> +            fenvf = fs.open_file(fenv)
> +            grubenv = fenvf.read(FS_READ_MAX)
> +            del fenvf
> +            if sys.version_info[0] < 3:
> +                self.cf.parse(buf, grubenv)
> +            else:
> +                self.cf.parse(buf.decode(), grubenv.decode())
> +        else:
> +            if sys.version_info[0] < 3:
> +                self.cf.parse(buf)
> +            else:
> +                self.cf.parse(buf.decode())

Can you please do something like

   grubenv = None
   if fenv ...
       ...

   if grubenv is not None and sys.version_info[0] < 3:
       grubenv = grubenv.decode()

and then you could at least avoid further multiplications of the call
to .parse...

Thanks,
Ian.

Patch

diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
index ce7ab0eb8c..53a0803817 100755
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -457,10 +457,25 @@  class Grub:
         # limit read size to avoid pathological cases
         buf = f.read(FS_READ_MAX)
         del f
-        if sys.version_info[0] < 3:
-            self.cf.parse(buf)
+        # check for a grubenv file next to the grub.cfg file
+        (fdir, fsep, ffile) = self.cf.filename.rpartition("/")
+        if fdir != "" and ffile == "grub.cfg":
+            fenv = fdir + "/grubenv"
         else:
-            self.cf.parse(buf.decode())
+            fenv = ""
+        if fenv != "" and fs.file_exists(fenv):
+            fenvf = fs.open_file(fenv)
+            grubenv = fenvf.read(FS_READ_MAX)
+            del fenvf
+            if sys.version_info[0] < 3:
+                self.cf.parse(buf, grubenv)
+            else:
+                self.cf.parse(buf.decode(), grubenv.decode())
+        else:
+            if sys.version_info[0] < 3:
+                self.cf.parse(buf)
+            else:
+                self.cf.parse(buf.decode())
 
     def image_index(self):
         if isinstance(self.cf.default, int):