Re: [PATCH 3/3] MdeModulePkg/BdsDxe: Avoid overwriting PlatformRecovery####

["Wang, Sunny (HPS SW)" <sunnywang@hpe.com>]

Hi Ray,
This patch looks good to me. 
Reviewed-by: Sunny Wang <sunnywang@hpe.com>

Just a suggestion. If we can add the code block below into BmGetFreeOptionNumber(), I think it would be better.

+  LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, 
+ LoadOptionTypePlatformRecovery);  if (EfiBootManagerFindLoadOption (&LoadOption, LoadOptions, LoadOptionCount) == -1) {
+    for (Index = 0; Index < LoadOptionCount; Index++) {
+      //
+      // The PlatformRecovery#### options are sorted by OptionNumber.
+      // Find the the smallest unused number as the new OptionNumber.
+      //
+      if (LoadOptions[Index].OptionNumber != Index) {
+        break;
+      }
+    }
+    LoadOption.OptionNumber = Index;
+    Status = EfiBootManagerLoadOptionToVariable (&LoadOption);
+    ASSERT_EFI_ERROR (Status);
+  }

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, November 15, 2016 6:04 PM
To: edk2-devel@lists.01.org
Cc: Jie Lin <jie.lin@intel.com>
Subject: [edk2] [PATCH 3/3] MdeModulePkg/BdsDxe: Avoid overwriting PlatformRecovery####

Current implementation always creates PlatformRecovery0000 pointing to \EFI\BOOT\BOOT$(ARCH).efi but it may overwrite PlatformRecovery#### created before (maybe by a DXE driver).

The patch only uses the smallest unused option number for the \EFI\BOOT\BOOT$(ARCH).efi PlatformRecovery#### to avoid overwriting already-created PlatformRecovery####.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jie Lin <jie.lin@intel.com>
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 48e5351..56034f5 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -834,7 +834,7 @@ BdsEntry (
   FilePath = FileDevicePath (NULL, EFI_REMOVABLE_MEDIA_FILE_NAME);
   Status = EfiBootManagerInitializeLoadOption (
              &LoadOption,
-             0,
+             LoadOptionNumberUnassigned,
              LoadOptionTypePlatformRecovery,
              LOAD_OPTION_ACTIVE,
              L"Default PlatformRecovery", @@ -843,9 +843,24 @@ BdsEntry (
              0
              );
   ASSERT_EFI_ERROR (Status);
-  EfiBootManagerLoadOptionToVariable (&LoadOption);
+  LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, 
+ LoadOptionTypePlatformRecovery);  if (EfiBootManagerFindLoadOption (&LoadOption, LoadOptions, LoadOptionCount) == -1) {
+    for (Index = 0; Index < LoadOptionCount; Index++) {
+      //
+      // The PlatformRecovery#### options are sorted by OptionNumber.
+      // Find the the smallest unused number as the new OptionNumber.
+      //
+      if (LoadOptions[Index].OptionNumber != Index) {
+        break;
+      }
+    }
+    LoadOption.OptionNumber = Index;
+    Status = EfiBootManagerLoadOptionToVariable (&LoadOption);
+    ASSERT_EFI_ERROR (Status);
+  }
   EfiBootManagerFreeLoadOption (&LoadOption);
   FreePool (FilePath);
+  EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
 
   //
   // Report Status Code to indicate connecting drivers will happen
--
2.9.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel