Thanks Ray. We will address the feedback and resubmit a new patch. From: edk2-devel [mailto:[email protected]] On Behalf Of Ni, Ruiyu Sent: Wednesday, March 30, 2016 7:39 PM To: El-Haj-Mahmoud, Samer <[email protected]>; Palmer, Thomas <[email protected]>; [email protected] Cc: Tian, Feng <[email protected]>; Zeng, Star <[email protected]> Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Expose BmLoadOption function
No there is no issue in exposing this function API. comments about this patch: 1. the patch title is "Expose BmLoadOption" which is confusing. 2. Internal function is called by internal code so some parameter checking can be ignored or just use assertion check. To make it public, the parameter checking needs to be added. a. VariableName should not be NULL b. OptionNumber, OptionType can be OPTIONAL both NULL means caller only needs to know whether the VariableName is valid but doesn't want to know the OptionNumber and OptionType. Regards, Ray From: El-Haj-Mahmoud, Samer [mailto:[email protected]] Sent: Wednesday, March 30, 2016 11:02 PM To: Ni, Ruiyu <[email protected]<mailto:[email protected]>>; Palmer, Thomas <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]> Cc: Tian, Feng <[email protected]<mailto:[email protected]>>; Zeng, Star <[email protected]<mailto:[email protected]>>; El-Haj-Mahmoud, Samer <[email protected]<mailto:[email protected]>>; Wang, Sunny (HPS SW) <[email protected]<mailto:[email protected]>> Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Expose BmLoadOption function There are use cases for it in platform customizations of BDS behavior... Is there an issue in exposing the function through the library API? -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Ni, Ruiyu Sent: Tuesday, March 29, 2016 7:52 PM To: El-Haj-Mahmoud, Samer <[email protected]<mailto:[email protected]>>; Palmer, Thomas <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]> Cc: Tian, Feng <[email protected]<mailto:[email protected]>>; Zeng, Star <[email protected]<mailto:[email protected]>> Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Expose BmLoadOption function Thomas, May I know the purpose to expose this function? Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:[email protected]] On Behalf Of >El-Haj-Mahmoud, Samer >Sent: Wednesday, March 30, 2016 6:12 AM >To: Palmer, Thomas <[email protected]<mailto:[email protected]>>; >[email protected]<mailto:[email protected]> >Cc: Tian, Feng <[email protected]<mailto:[email protected]>>; Zeng, Star ><[email protected]<mailto:[email protected]>> >Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Expose >BmLoadOption function > >Reviewed-by: Samer El-Haj-Mahmoud <[email protected]<mailto:[email protected]>> > >-----Original Message----- >From: Palmer, Thomas >Sent: Tuesday, March 29, 2016 5:08 PM >To: [email protected]<mailto:[email protected]> >Cc: El-Haj-Mahmoud, Samer ><[email protected]<mailto:[email protected]>>; >[email protected]<mailto:[email protected]>; >[email protected]<mailto:[email protected]>; Palmer, Thomas ><[email protected]<mailto:[email protected]>> >Subject: [PATCH] MdeModulePkg/UefiBootManagerLib: Expose BmLoadOption >function > >Redfine the BmIsValidLoadOptionVariableName function to allow public >use. Change name to EfiBootManagerIsValidLoadOptionVariableName to match >naming scheme. > >Contributed-under: TianoCore Contribution Agreement 1.0 >Signed-off-by: Thomas Palmer ><[email protected]<mailto:[email protected]>> >--- > MdeModulePkg/Include/Library/UefiBootManagerLib.h | 23 +++++++++++++++++++++- > .../Library/UefiBootManagerLib/BmLoadOption.c | 9 +++++---- > 2 files changed, 27 insertions(+), 5 deletions(-) > >diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h >b/MdeModulePkg/Include/Library/UefiBootManagerLib.h >index afb4271..e761ef2 100644 >--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h >+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h >@@ -2,7 +2,7 @@ > Provide Boot Manager related library APIs. > > Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.<BR> >-(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR> >+(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR> > This program and the accompanying materials are licensed and made >available under the terms and conditions of the BSD License which >accompanies this distribution. The full text of the license may be found at >@@ -731,4 +731,25 @@ EFIAPI EfiBootManagerProcessLoadOption ( > EFI_BOOT_MANAGER_LOAD_OPTION *LoadOption > ); >+ >+/** >+ Check whether the VariableName is a valid load option variable name >+ and return the load option type and option number. >+ >+ @param VariableName The name of the load option variable. >+ @param OptionType Return the load option type. >+ @param OptionNumber Return the load option number. >+ >+ @retval TRUE The variable name is valid; The load option type and >+ load option number is returned. >+ @retval FALSE The variable name is NOT valid. >+**/ >+BOOLEAN >+EFIAPI >+EfiBootManagerIsValidLoadOptionVariableName ( >+ IN CHAR16 *VariableName, >+ OUT EFI_BOOT_MANAGER_LOAD_OPTION_TYPE *OptionType, >+ OUT UINT16 *OptionNumber >+ ); >+ > #endif >diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c >b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c >index 696e995..20fe6af 100644 >--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c >+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c >@@ -2,7 +2,7 @@ > Load option library functions which relate with creating and processing > load options. > > Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR> >-(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR> >+(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR> > This program and the accompanying materials are licensed and made >available under the terms and conditions of the BSD License which >accompanies this distribution. The full text of the license may be found at >@@ -775,7 +775,8 @@ BmValidateOption ( > @retval FALSE The variable name is NOT valid. > **/ > BOOLEAN >-BmIsValidLoadOptionVariableName ( >+EFIAPI >+EfiBootManagerIsValidLoadOptionVariableName ( > IN CHAR16 *VariableName, > OUT EFI_BOOT_MANAGER_LOAD_OPTION_TYPE *OptionType, > OUT UINT16 *OptionNumber >@@ -853,7 +854,7 @@ EfiBootManagerVariableToLoadOptionEx ( > return EFI_INVALID_PARAMETER; > } > >- if (!BmIsValidLoadOptionVariableName (VariableName, &OptionType, >&OptionNumber)) { >+ if (!EfiBootManagerIsValidLoadOptionVariableName (VariableName, >+ &OptionType, &OptionNumber)) { > return EFI_INVALID_PARAMETER; > } > >@@ -979,7 +980,7 @@ BmCollectLoadOptions ( > > if (CompareGuid (Guid, Param->Guid) && ( > Param->OptionType == LoadOptionTypePlatformRecovery && >- BmIsValidLoadOptionVariableName (Name, &OptionType, &OptionNumber) && >+ EfiBootManagerIsValidLoadOptionVariableName (Name, &OptionType, >+ &OptionNumber) && > OptionType == LoadOptionTypePlatformRecovery > )) { > Status = EfiBootManagerVariableToLoadOptionEx (Name, Guid, >&Option); >-- >1.9.1 > >_______________________________________________ >edk2-devel mailing list >[email protected]<mailto:[email protected]> >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected]<mailto:[email protected]> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
