diff mbox series

[edk2,1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Update RouteConfig function

Message ID 1524083495-31936-1-git-send-email-thomas.palmer@hpe.com
State Accepted
Commit a82385958e6e5ffe8151ad71f0a47cb06e2d5b20
Headers show
Series [edk2,1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Update RouteConfig function | expand

Commit Message

Palmer, Thomas April 18, 2018, 8:31 p.m. UTC 
According to UEFI spec, the RouteConfig protocol function should populate
the Progress pointer with an address inside Configuration.  This patch
ensures that these functions are compliant when EFI_NOT_FOUND is returned.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Thomas Palmer <thomas.palmer@hpe.com>

---
 .../Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c                   | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.7.4

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

Comments

Bi, Dandan April 20, 2018, 7:34 a.m. UTC | #1 
Thanks for the updating. These changes make sense.
Reviewed-by: Dandan Bi <dandan.bi@intel.com>  for this patch series.


But the Spec seems not to be clear enough.
When looking into details about the "progress" parameter in EFI HII Configuration Routing Protocol and  EFI_HII_CONFIG_ACCESS_PROTOCOL.

Description of "progress" parameter in ExtractConfig() in UEFI 2.7 Spec:
Progress
On return, points to a character in the Request string. Points to the string's null terminator if request was successful. Points to the most recent '&' before the first failing name / value pair (or the beginning of the string if the failure is in the first name / value pair) if the request was not successful
	
EFI_NOT_FOUND
A configuration element matching the routing data is not found. Progress set to the first character in the routing header.


Description of "progress" parameter in RouteConfig () in UEFI 2.7 Spec:
Progress
a pointer to a string filled in with the offset of the most recent '&' before the first failing name / value pair (or the beginning of the string if the failure is in the first name / value pair) or the terminating NULL if all was successful.

EFI_NOT_FOUND
Target for the specified routing data was not found.

Compared with ExtractConfig(), the description of "Progress" parameter in RouteConfig()  is not very clear. 
We think an ECR is nice to have to clarify them. What do you think?


Thanks,
Dandan

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Thomas Palmer

Sent: Thursday, April 19, 2018 4:32 AM
To: edk2-devel@lists.01.org
Cc: nickle.wang@hpe.com; Gao, Liming <liming.gao@intel.com>
Subject: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Update RouteConfig function

According to UEFI spec, the RouteConfig protocol function should populate the Progress pointer with an address inside Configuration.  This patch ensures that these functions are compliant when EFI_NOT_FOUND is returned.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Thomas Palmer <thomas.palmer@hpe.com>

---
 .../Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c                   | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
index a4828b7130c7..3092184ab760 100644
--- a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
+++ b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMai
+++ ntUi.c
@@ -2,6 +2,7 @@
   Legacy Boot Maintainence UI implementation.
 
 Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2018 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 @@ -563,6 +564,8 @@ LegacyBootOptionRouteConfig (
     return EFI_INVALID_PARAMETER;
   }
 
+  *Progress = Configuration;
+
   //
   // Check routing data in <ConfigHdr>.
   // Note: there is no name for Name/Value storage, only GUID will be checked
--
2.7.4

_______________________________________________
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
Palmer, Thomas April 20, 2018, 4 p.m. UTC | #2 
I have no opinion / please the 5th.  I defer to the experts.


Regards,

Thomas Palmer

"I have only made this letter longer because I have not had the time to make it shorter" - Blaise Pascal


-----Original Message-----
From: Bi, Dandan [mailto:dandan.bi@intel.com] 

Sent: Friday, April 20, 2018 2:34 AM
To: Palmer, Thomas <thomas.palmer@hpe.com>; edk2-devel@lists.01.org
Cc: Wang, Nickle (HPS SW) <nickle.wang@hpe.com>; Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: RE: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Update RouteConfig function

Thanks for the updating. These changes make sense.
Reviewed-by: Dandan Bi <dandan.bi@intel.com>  for this patch series.


But the Spec seems not to be clear enough.
When looking into details about the "progress" parameter in EFI HII Configuration Routing Protocol and  EFI_HII_CONFIG_ACCESS_PROTOCOL.

Description of "progress" parameter in ExtractConfig() in UEFI 2.7 Spec:
Progress
On return, points to a character in the Request string. Points to the string's null terminator if request was successful. Points to the most recent '&' before the first failing name / value pair (or the beginning of the string if the failure is in the first name / value pair) if the request was not successful
	
EFI_NOT_FOUND
A configuration element matching the routing data is not found. Progress set to the first character in the routing header.


Description of "progress" parameter in RouteConfig () in UEFI 2.7 Spec:
Progress
a pointer to a string filled in with the offset of the most recent '&' before the first failing name / value pair (or the beginning of the string if the failure is in the first name / value pair) or the terminating NULL if all was successful.

EFI_NOT_FOUND
Target for the specified routing data was not found.

Compared with ExtractConfig(), the description of "Progress" parameter in RouteConfig()  is not very clear. 
We think an ECR is nice to have to clarify them. What do you think?


Thanks,
Dandan

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Thomas Palmer

Sent: Thursday, April 19, 2018 4:32 AM
To: edk2-devel@lists.01.org
Cc: nickle.wang@hpe.com; Gao, Liming <liming.gao@intel.com>
Subject: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Update RouteConfig function

According to UEFI spec, the RouteConfig protocol function should populate the Progress pointer with an address inside Configuration.  This patch ensures that these functions are compliant when EFI_NOT_FOUND is returned.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Thomas Palmer <thomas.palmer@hpe.com>

---
 .../Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c                   | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
index a4828b7130c7..3092184ab760 100644
--- a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
+++ b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMai
+++ ntUi.c
@@ -2,6 +2,7 @@
   Legacy Boot Maintainence UI implementation.
 
 Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2018 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 @@ -563,6 +564,8 @@ LegacyBootOptionRouteConfig (
     return EFI_INVALID_PARAMETER;
   }
 
+  *Progress = Configuration;
+
   //
   // Check routing data in <ConfigHdr>.
   // Note: there is no name for Name/Value storage, only GUID will be checked
--
2.7.4

_______________________________________________
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
Dong, Eric May 3, 2018, 2:55 a.m. UTC | #3 
Hi Thomas,

Thanks for your patches. We just finished internal verification of them.

Reviewed-by: Eric Dong <eric.dong@intel.com> and pushed them.


Thanks,
Eric

-----Original Message-----
From: Palmer, Thomas [mailto:thomas.palmer@hpe.com] 

Sent: Saturday, April 21, 2018 12:00 AM
To: Bi, Dandan <dandan.bi@intel.com>; edk2-devel@lists.01.org
Cc: Wang, Nickle (HPS SW) <nickle.wang@hpe.com>; Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: RE: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Update RouteConfig function

I have no opinion / please the 5th.  I defer to the experts.


Regards,

Thomas Palmer

"I have only made this letter longer because I have not had the time to make it shorter" - Blaise Pascal


-----Original Message-----
From: Bi, Dandan [mailto:dandan.bi@intel.com] 

Sent: Friday, April 20, 2018 2:34 AM
To: Palmer, Thomas <thomas.palmer@hpe.com>; edk2-devel@lists.01.org
Cc: Wang, Nickle (HPS SW) <nickle.wang@hpe.com>; Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: RE: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Update RouteConfig function

Thanks for the updating. These changes make sense.
Reviewed-by: Dandan Bi <dandan.bi@intel.com>  for this patch series.


But the Spec seems not to be clear enough.
When looking into details about the "progress" parameter in EFI HII Configuration Routing Protocol and  EFI_HII_CONFIG_ACCESS_PROTOCOL.

Description of "progress" parameter in ExtractConfig() in UEFI 2.7 Spec:
Progress
On return, points to a character in the Request string. Points to the string's null terminator if request was successful. Points to the most recent '&' before the first failing name / value pair (or the beginning of the string if the failure is in the first name / value pair) if the request was not successful

EFI_NOT_FOUND
A configuration element matching the routing data is not found. Progress set to the first character in the routing header.


Description of "progress" parameter in RouteConfig () in UEFI 2.7 Spec:
Progress
a pointer to a string filled in with the offset of the most recent '&' before the first failing name / value pair (or the beginning of the string if the failure is in the first name / value pair) or the terminating NULL if all was successful.

EFI_NOT_FOUND
Target for the specified routing data was not found.

Compared with ExtractConfig(), the description of "Progress" parameter in RouteConfig()  is not very clear. 
We think an ECR is nice to have to clarify them. What do you think?


Thanks,
Dandan

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Thomas Palmer

Sent: Thursday, April 19, 2018 4:32 AM
To: edk2-devel@lists.01.org
Cc: nickle.wang@hpe.com; Gao, Liming <liming.gao@intel.com>
Subject: [edk2] [PATCH 1/8] IntelFrameworkModulePkg/LegacyBootMaintUiLib: Update RouteConfig function

According to UEFI spec, the RouteConfig protocol function should populate the Progress pointer with an address inside Configuration.  This patch ensures that these functions are compliant when EFI_NOT_FOUND is returned.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Thomas Palmer <thomas.palmer@hpe.com>

---
 .../Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c                   | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
index a4828b7130c7..3092184ab760 100644
--- a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
+++ b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMai
+++ ntUi.c
@@ -2,6 +2,7 @@
   Legacy Boot Maintainence UI implementation.
 
 Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2018 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 @@ -563,6 +564,8 @@ LegacyBootOptionRouteConfig (
     return EFI_INVALID_PARAMETER;
   }
 
+  *Progress = Configuration;
+
   //
   // Check routing data in <ConfigHdr>.
   // Note: there is no name for Name/Value storage, only GUID will be checked
--
2.7.4

_______________________________________________
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
diff mbox series

Patch

diff --git a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
index a4828b7130c7..3092184ab760 100644
--- a/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
+++ b/IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
@@ -2,6 +2,7 @@ 
   Legacy Boot Maintainence UI implementation.
 
 Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2018 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
@@ -563,6 +564,8 @@  LegacyBootOptionRouteConfig (
     return EFI_INVALID_PARAMETER;
   }
 
+  *Progress = Configuration;
+
   //
   // Check routing data in <ConfigHdr>.
   // Note: there is no name for Name/Value storage, only GUID will be checked