* [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
@ 2018-04-17 9:30 Laszlo Ersek
2018-04-17 10:10 ` Laszlo Ersek
2018-04-20 14:55 ` Laszlo Ersek
0 siblings, 2 replies; 6+ messages in thread
From: Laszlo Ersek @ 2018-04-17 9:30 UTC (permalink / raw)
To: edk2-devel; +Cc: Eric Dong, Ruiyu Ni, Star Zeng
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
2018-04-17 9:30 [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype Laszlo Ersek
@ 2018-04-17 10:10 ` Laszlo Ersek
2018-04-20 14:55 ` Laszlo Ersek
1 sibling, 0 replies; 6+ messages in thread
From: Laszlo Ersek @ 2018-04-17 10:10 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Eric Dong, Star Zeng
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
2018-04-17 9:30 [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype Laszlo Ersek
2018-04-17 10:10 ` Laszlo Ersek
@ 2018-04-20 14:55 ` Laszlo Ersek
2018-04-23 3:24 ` Zeng, Star
1 sibling, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2018-04-20 14:55 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Eric Dong, Star Zeng
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;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
2018-04-20 14:55 ` Laszlo Ersek
@ 2018-04-23 3:24 ` Zeng, Star
2018-04-23 18:48 ` Laszlo Ersek
0 siblings, 1 reply; 6+ messages in thread
From: Zeng, Star @ 2018-04-23 3:24 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Dong, Eric, Zeng, Star
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;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
2018-04-23 3:24 ` Zeng, Star
@ 2018-04-23 18:48 ` Laszlo Ersek
2018-04-24 0:43 ` Zeng, Star
0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2018-04-23 18:48 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Dong, Eric
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype
2018-04-23 18:48 ` Laszlo Ersek
@ 2018-04-24 0:43 ` Zeng, Star
0 siblings, 0 replies; 6+ messages in thread
From: Zeng, Star @ 2018-04-24 0:43 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Dong, Eric, Zeng, Star
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-24 0:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-17 9:30 [PATCH] MdeModulePkg/UefiBootManagerLib: fix AddLoadOptionVariable docs/prototype Laszlo Ersek
2018-04-17 10:10 ` Laszlo Ersek
2018-04-20 14:55 ` Laszlo Ersek
2018-04-23 3:24 ` Zeng, Star
2018-04-23 18:48 ` Laszlo Ersek
2018-04-24 0:43 ` Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox