[edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype

Laszlo Ersek posted 1 patch 6 years ago
Failed in applying to current master (apply log)
MdeModulePkg/Include/Library/UefiBootManagerLib.h  | 26 +++++++++++++++-------
.../Library/UefiBootManagerLib/BmLoadOption.c      | 12 ++++++----
2 files changed, 26 insertions(+), 12 deletions(-)
[edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
Posted by Laszlo Ersek 6 years ago
Clean up the leading comment and the prototype of
EfiBootManagerAddLoadOptionVariable():

- the function may modify Option on output, annotate the parameter with
  OUT and update the documentation;

- "@retval EFI_STATUS" and "@retval Others" are not idiomatic
  documentation, use @return instead;

- sync comment and prototype between lib instance and lib class header.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    Repo:   https://github.com/lersek/edk2.git
    Branch: add_load_opt_inout

 MdeModulePkg/Include/Library/UefiBootManagerLib.h  | 26 +++++++++++++++-------
 .../Library/UefiBootManagerLib/BmLoadOption.c      | 12 ++++++----
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index 97ac1f233ce9..1d862a4b2684 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -176,20 +176,30 @@ EfiBootManagerLoadOptionToVariable (
   );
 
 /**
-  This function will update the Boot####/Driver####/SysPrep#### and the 
-  BootOrder/DriverOrder/SysPrepOrder to add a new load option.
+  This function will register the new Boot####, Driver#### or SysPrep#### option.
+  After the *#### is updated, the *Order will also be updated.
 
-  @param  Option        Pointer to load option to add.
-  @param  Position      Position of the new load option to put in the BootOrder/DriverOrder/SysPrepOrder.
+  @param  Option            Pointer to load option to add. If on input
+                            Option->OptionNumber is LoadOptionNumberUnassigned,
+                            then on output Option->OptionNumber is updated to
+                            the number of the new Boot####,
+                            Driver#### or SysPrep#### option.
+  @param  Position          Position of the new load option to put in the ****Order variable.
+
+  @retval EFI_SUCCESS           The *#### have been successfully registered.
+  @retval EFI_INVALID_PARAMETER The option number exceeds 0xFFFF.
+  @retval EFI_ALREADY_STARTED   The option number of Option is being used already.
+                                Note: this API only adds new load option, no replacement support.
+  @retval EFI_OUT_OF_RESOURCES  There is no free option number that can be used when the
+                                option number specified in the Option is LoadOptionNumberUnassigned.
+  @return                       Status codes of gRT->SetVariable ().
 
-  @retval EFI_SUCCESS   The load option has been successfully added.
-  @retval Others        Error status returned by RT->SetVariable.
 **/
 EFI_STATUS
 EFIAPI
 EfiBootManagerAddLoadOptionVariable (
-  IN EFI_BOOT_MANAGER_LOAD_OPTION  *Option,
-  IN UINTN                         Position
+  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION *Option,
+  IN     UINTN                        Position
   );
 
 /**
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 32918caf324c..f88f8e02451c 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -329,7 +329,11 @@ BmAddOptionNumberToOrderVariable (
   This function will register the new Boot####, Driver#### or SysPrep#### option.
   After the *#### is updated, the *Order will also be updated.
 
-  @param  Option            Pointer to load option to add.
+  @param  Option            Pointer to load option to add. If on input
+                            Option->OptionNumber is LoadOptionNumberUnassigned,
+                            then on output Option->OptionNumber is updated to
+                            the number of the new Boot####,
+                            Driver#### or SysPrep#### option.
   @param  Position          Position of the new load option to put in the ****Order variable.
 
   @retval EFI_SUCCESS           The *#### have been successfully registered.
@@ -338,14 +342,14 @@ BmAddOptionNumberToOrderVariable (
                                 Note: this API only adds new load option, no replacement support.
   @retval EFI_OUT_OF_RESOURCES  There is no free option number that can be used when the
                                 option number specified in the Option is LoadOptionNumberUnassigned.
-  @retval EFI_STATUS            Return the status of gRT->SetVariable ().
+  @return                       Status codes of gRT->SetVariable ().
 
 **/
 EFI_STATUS
 EFIAPI
 EfiBootManagerAddLoadOptionVariable (
-  IN EFI_BOOT_MANAGER_LOAD_OPTION *Option,
-  IN UINTN                        Position
+  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION *Option,
+  IN     UINTN                        Position
   )
 {
   EFI_STATUS                      Status;
-- 
2.14.1.3.gb7cf6e02401b

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
Posted by Laszlo Ersek 6 years ago
On 04/17/18 11:30, Laszlo Ersek wrote:

>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  | 26 +++++++++++++++-------
>  .../Library/UefiBootManagerLib/BmLoadOption.c      | 12 ++++++----
>  2 files changed, 26 insertions(+), 12 deletions(-)

Sorry about the truncated diffstat, I forgot to add "--stat=1000
--stat-graph-width=20" to the git-format-patch cmdline. More sleep
needed. :/

Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
Posted by Laszlo Ersek 6 years ago
Ping -- can I get a review for this please; it's a trivial patch.

Thanks
Laszlo

On 04/17/18 11:30, Laszlo Ersek wrote:
> Clean up the leading comment and the prototype of
> EfiBootManagerAddLoadOptionVariable():
> 
> - the function may modify Option on output, annotate the parameter with
>   OUT and update the documentation;
> 
> - "@retval EFI_STATUS" and "@retval Others" are not idiomatic
>   documentation, use @return instead;
> 
> - sync comment and prototype between lib instance and lib class header.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     Repo:   https://github.com/lersek/edk2.git
>     Branch: add_load_opt_inout
> 
>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  | 26 +++++++++++++++-------
>  .../Library/UefiBootManagerLib/BmLoadOption.c      | 12 ++++++----
>  2 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> index 97ac1f233ce9..1d862a4b2684 100644
> --- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> +++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> @@ -176,20 +176,30 @@ EfiBootManagerLoadOptionToVariable (
>    );
>  
>  /**
> -  This function will update the Boot####/Driver####/SysPrep#### and the 
> -  BootOrder/DriverOrder/SysPrepOrder to add a new load option.
> +  This function will register the new Boot####, Driver#### or SysPrep#### option.
> +  After the *#### is updated, the *Order will also be updated.
>  
> -  @param  Option        Pointer to load option to add.
> -  @param  Position      Position of the new load option to put in the BootOrder/DriverOrder/SysPrepOrder.
> +  @param  Option            Pointer to load option to add. If on input
> +                            Option->OptionNumber is LoadOptionNumberUnassigned,
> +                            then on output Option->OptionNumber is updated to
> +                            the number of the new Boot####,
> +                            Driver#### or SysPrep#### option.
> +  @param  Position          Position of the new load option to put in the ****Order variable.
> +
> +  @retval EFI_SUCCESS           The *#### have been successfully registered.
> +  @retval EFI_INVALID_PARAMETER The option number exceeds 0xFFFF.
> +  @retval EFI_ALREADY_STARTED   The option number of Option is being used already.
> +                                Note: this API only adds new load option, no replacement support.
> +  @retval EFI_OUT_OF_RESOURCES  There is no free option number that can be used when the
> +                                option number specified in the Option is LoadOptionNumberUnassigned.
> +  @return                       Status codes of gRT->SetVariable ().
>  
> -  @retval EFI_SUCCESS   The load option has been successfully added.
> -  @retval Others        Error status returned by RT->SetVariable.
>  **/
>  EFI_STATUS
>  EFIAPI
>  EfiBootManagerAddLoadOptionVariable (
> -  IN EFI_BOOT_MANAGER_LOAD_OPTION  *Option,
> -  IN UINTN                         Position
> +  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION *Option,
> +  IN     UINTN                        Position
>    );
>  
>  /**
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> index 32918caf324c..f88f8e02451c 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> @@ -329,7 +329,11 @@ BmAddOptionNumberToOrderVariable (
>    This function will register the new Boot####, Driver#### or SysPrep#### option.
>    After the *#### is updated, the *Order will also be updated.
>  
> -  @param  Option            Pointer to load option to add.
> +  @param  Option            Pointer to load option to add. If on input
> +                            Option->OptionNumber is LoadOptionNumberUnassigned,
> +                            then on output Option->OptionNumber is updated to
> +                            the number of the new Boot####,
> +                            Driver#### or SysPrep#### option.
>    @param  Position          Position of the new load option to put in the ****Order variable.
>  
>    @retval EFI_SUCCESS           The *#### have been successfully registered.
> @@ -338,14 +342,14 @@ BmAddOptionNumberToOrderVariable (
>                                  Note: this API only adds new load option, no replacement support.
>    @retval EFI_OUT_OF_RESOURCES  There is no free option number that can be used when the
>                                  option number specified in the Option is LoadOptionNumberUnassigned.
> -  @retval EFI_STATUS            Return the status of gRT->SetVariable ().
> +  @return                       Status codes of gRT->SetVariable ().
>  
>  **/
>  EFI_STATUS
>  EFIAPI
>  EfiBootManagerAddLoadOptionVariable (
> -  IN EFI_BOOT_MANAGER_LOAD_OPTION *Option,
> -  IN UINTN                        Position
> +  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION *Option,
> +  IN     UINTN                        Position
>    )
>  {
>    EFI_STATUS                      Status;
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
Posted by Zeng, Star 6 years ago
Reviewed-by: Star Zeng <star.zeng@intel.com>


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Friday, April 20, 2018 10:56 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype

Ping -- can I get a review for this please; it's a trivial patch.

Thanks
Laszlo

On 04/17/18 11:30, Laszlo Ersek wrote:
> Clean up the leading comment and the prototype of
> EfiBootManagerAddLoadOptionVariable():
> 
> - the function may modify Option on output, annotate the parameter with
>   OUT and update the documentation;
> 
> - "@retval EFI_STATUS" and "@retval Others" are not idiomatic
>   documentation, use @return instead;
> 
> - sync comment and prototype between lib instance and lib class header.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     Repo:   https://github.com/lersek/edk2.git
>     Branch: add_load_opt_inout
> 
>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  | 26 +++++++++++++++-------
>  .../Library/UefiBootManagerLib/BmLoadOption.c      | 12 ++++++----
>  2 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> index 97ac1f233ce9..1d862a4b2684 100644
> --- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> +++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> @@ -176,20 +176,30 @@ EfiBootManagerLoadOptionToVariable (
>    );
>  
>  /**
> -  This function will update the Boot####/Driver####/SysPrep#### and the 
> -  BootOrder/DriverOrder/SysPrepOrder to add a new load option.
> +  This function will register the new Boot####, Driver#### or SysPrep#### option.
> +  After the *#### is updated, the *Order will also be updated.
>  
> -  @param  Option        Pointer to load option to add.
> -  @param  Position      Position of the new load option to put in the BootOrder/DriverOrder/SysPrepOrder.
> +  @param  Option            Pointer to load option to add. If on input
> +                            Option->OptionNumber is LoadOptionNumberUnassigned,
> +                            then on output Option->OptionNumber is updated to
> +                            the number of the new Boot####,
> +                            Driver#### or SysPrep#### option.
> +  @param  Position          Position of the new load option to put in the ****Order variable.
> +
> +  @retval EFI_SUCCESS           The *#### have been successfully registered.
> +  @retval EFI_INVALID_PARAMETER The option number exceeds 0xFFFF.
> +  @retval EFI_ALREADY_STARTED   The option number of Option is being used already.
> +                                Note: this API only adds new load option, no replacement support.
> +  @retval EFI_OUT_OF_RESOURCES  There is no free option number that can be used when the
> +                                option number specified in the Option is LoadOptionNumberUnassigned.
> +  @return                       Status codes of gRT->SetVariable ().
>  
> -  @retval EFI_SUCCESS   The load option has been successfully added.
> -  @retval Others        Error status returned by RT->SetVariable.
>  **/
>  EFI_STATUS
>  EFIAPI
>  EfiBootManagerAddLoadOptionVariable (
> -  IN EFI_BOOT_MANAGER_LOAD_OPTION  *Option,
> -  IN UINTN                         Position
> +  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION *Option,
> +  IN     UINTN                        Position
>    );
>  
>  /**
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> index 32918caf324c..f88f8e02451c 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> @@ -329,7 +329,11 @@ BmAddOptionNumberToOrderVariable (
>    This function will register the new Boot####, Driver#### or SysPrep#### option.
>    After the *#### is updated, the *Order will also be updated.
>  
> -  @param  Option            Pointer to load option to add.
> +  @param  Option            Pointer to load option to add. If on input
> +                            Option->OptionNumber is LoadOptionNumberUnassigned,
> +                            then on output Option->OptionNumber is updated to
> +                            the number of the new Boot####,
> +                            Driver#### or SysPrep#### option.
>    @param  Position          Position of the new load option to put in the ****Order variable.
>  
>    @retval EFI_SUCCESS           The *#### have been successfully registered.
> @@ -338,14 +342,14 @@ BmAddOptionNumberToOrderVariable (
>                                  Note: this API only adds new load option, no replacement support.
>    @retval EFI_OUT_OF_RESOURCES  There is no free option number that can be used when the
>                                  option number specified in the Option is LoadOptionNumberUnassigned.
> -  @retval EFI_STATUS            Return the status of gRT->SetVariable ().
> +  @return                       Status codes of gRT->SetVariable ().
>  
>  **/
>  EFI_STATUS
>  EFIAPI
>  EfiBootManagerAddLoadOptionVariable (
> -  IN EFI_BOOT_MANAGER_LOAD_OPTION *Option,
> -  IN UINTN                        Position
> +  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION *Option,
> +  IN     UINTN                        Position
>    )
>  {
>    EFI_STATUS                      Status;
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
Posted by Laszlo Ersek 6 years ago
On 04/23/18 05:24, Zeng, Star wrote:
> Reviewed-by: Star Zeng <star.zeng@intel.com>

Thanks, Star -- I intended to push the patch now, but then I noticed Ray
pushed it already (commit 8b5c80e0296c). Thanks!

Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
Posted by Zeng, Star 6 years ago
That is fine. :)

Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Tuesday, April 24, 2018 2:48 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype

On 04/23/18 05:24, Zeng, Star wrote:
> Reviewed-by: Star Zeng <star.zeng@intel.com>

Thanks, Star -- I intended to push the patch now, but then I noticed Ray pushed it already (commit 8b5c80e0296c). Thanks!

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