Regards, Ray
>-----Original Message----- >From: Laszlo Ersek [mailto:ler...@redhat.com] >Sent: Monday, May 2, 2016 6:25 PM >To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-de...@ml01.01.org >Cc: Justen, Jordan L <jordan.l.jus...@intel.com> >Subject: Re: [edk2] [Patch v4 04/23] OvmfPkg/QemuNewBootOrderLib: Build with >UefiBootManagerLib > >Hi Ray, > >My v3 review for this patch is here: > >http://news.gmane.org/find-root.php?message_id=%3c571f82d8.7010...@redhat.com%3E > >* You didn't answer my question in (2) -- can you please answer it now? > >(The question was if EFI_BOOT_MANAGER_LOAD_OPTION.OptionNumber >corresponds to the number of the Boot#### variable that the >EFI_BOOT_MANAGER_LOAD_OPTION object was constructed from.) > >Furthermore, > >On 04/29/16 09:00, Ruiyu Ni wrote: >> @@ -308,22 +310,25 @@ BootOrderAppend ( >> >> /** >> >> - Create an array of ACTIVE_OPTION elements for a boot option list. >> + Create an array of ACTIVE_OPTION elements for a boot option array. >> >> - @param[in] BootOptionList A boot option list, created with >> - BdsLibEnumerateAllBootOption(). >> + @param[in] BootOptions A boot option array, created with >> + EfiBootManagerRefreshAllBootOptions () and >> + EfiBootManagerGetLoadOptions (). > >* The function name is EfiBootManagerRefreshAllBootOption, not >EfiBootManagerRefreshAllBootOptions[s]. > >Of course I wouldn't obsess about it, if it was the only remaining thing >to address in this patch; but it's not the only thing. Namely: > >> @@ -1454,31 +1458,26 @@ Match ( >> >> // >> // Attempt to expand any relative UEFI device path starting with HD() to >> an >> - // absolute device path first. The logic imitates >> BdsLibBootViaBootOption(). >> - // We don't have to free the absolute device path, >> - // BdsExpandPartitionPartialDevicePathToFull() has internal caching. >> + // absolute device path first. >> // >> - Result = FALSE; > >* You missed point (5) in my v3 review: this assignment should not be >removed. Otherwise, if the EfiBootManagerGetLoadOptionBuffer() call >fails, we jump to the Exit label with the Result variable uninitialized. > > >Please >- confirm (2), Yes. EFI_BOOT_MANAGER_LOAD_OPTION.OptionNumber is extracted from the load option name. For example, for Boot0005, OptionNumber is 5; for Driver0010, OptionNumber is 0x10. >- fix (5), OK I will add the Result = FALSE. >- fix the new typo, Sure. > >and then you can add my > >Reviewed-by: Laszlo Ersek <ler...@redhat.com> > >Thanks >Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
