Re: [PATCH] trivial fixes for windows-efi branch

[Andrey Borzenkov <arvidjaar@gmail.com>]

В Sun, 15 Dec 2013 12:43:49 +0100
"Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> пишет:

> Sounds like you're looking at old version. I've fixed those problems and
> it's already merged in master. Could you have a look and say if any of
> those still a problem?

Yes, it fixed file path and variable not existed in exhaustive search.
Still see comments below.

diff --git a/grub-core/osdep/windows/platform.c
b/grub-core/osdep/windows/platform.c index b123256..3f4ad5e 100644
--- a/grub-core/osdep/windows/platform.c
+++ b/grub-core/osdep/windows/platform.c
@@ -246,6 +246,8 @@ grub_install_register_efi (grub_device_t
efidir_grub_dev, void *current = NULL;
 	  ssize_t current_len;
 	  current = get_efi_variable_bootn (i, &current_len);
+	  if (current_len < 0)
+	    continue; /* FIXME Should we abort on error? */
 	  if (current_len < (distrib16_len + 1) * sizeof
(grub_uint16_t)
 	      + 6)
 	    {

Same potential problem with signed vs. unsigned comparison. Variable is
not guaranteed to exist here and -1 is treated as very large unsigned
value.

@@ -275,13 +277,18 @@ grub_install_register_efi (grub_device_t
efidir_grub_dev, void *current = NULL;
 	  ssize_t current_len;
 	  current = get_efi_variable_bootn (i, &current_len);
+	  if (current_len < -1)
+	    continue; /* FIXME Should we abort on error? */

For completeness we should handle failures to avoid crash.

 	  if (current_len == -1)
 	    {
-	      order_num = i;
-	      have_order_num = 1;
-	      grub_util_info ("Creating new entry at Boot%04x",
-			      order_num);
-	      break;
+	      if (!have_order_num)
+		{
+		  order_num = i;
+		  have_order_num = 1;
+		  grub_util_info ("Creating new entry at Boot%04x",
+				  order_num);
+		}
+	      continue;
 	    }

You changed it to stop on first non-existing variable; but this can
miss another one later with the same description. This will result in
two different boot entries with the same name.

I cannot test it on Windows; on Linux code appears to be fast enough.

>                       As for merging with Linux part we could just have
> primitives set variable and get variable and have everything else shared

Yes, I'll get a look.