* [PATCH 1/2] MdeModulePkg Variable: Failed to set a variable if the remaining size is equal to the variable data size.
@ 2018-05-11 4:17 cinnamon shia
2018-05-11 4:17 ` [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore() cinnamon shia
2018-05-11 8:26 ` [PATCH 1/2] MdeModulePkg Variable: Failed to set a variable if the remaining size is equal to the variable data size Zeng, Star
0 siblings, 2 replies; 7+ messages in thread
From: cinnamon shia @ 2018-05-11 4:17 UTC (permalink / raw)
To: edk2-devel; +Cc: lersek, star.zeng, cinnamon shia, Ansen Huang
Fix the issue that failed to update or add a UEFI variable if the remaining size is equal to the data size
of the variable.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 6caf603b3d..7303681aaa 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -17,7 +17,7 @@
integer overflow. It should also check attribute to avoid authentication bypass.
Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+(C) Copyright 2015-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
@@ -288,7 +288,7 @@ UpdateVariableStore (
DataPtr += mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase;
}
- if ((DataPtr + DataSize) >= ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
+ if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
return EFI_INVALID_PARAMETER;
}
} else {
@@ -301,7 +301,7 @@ UpdateVariableStore (
DataPtr += mVariableModuleGlobal->VariableGlobal.VolatileVariableBase;
}
- if ((DataPtr + DataSize) >= ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
+ if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
return EFI_INVALID_PARAMETER;
}
--
2.16.1.windows.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
2018-05-11 4:17 [PATCH 1/2] MdeModulePkg Variable: Failed to set a variable if the remaining size is equal to the variable data size cinnamon shia
@ 2018-05-11 4:17 ` cinnamon shia
2018-05-11 8:37 ` Zeng, Star
2018-05-11 8:26 ` [PATCH 1/2] MdeModulePkg Variable: Failed to set a variable if the remaining size is equal to the variable data size Zeng, Star
1 sibling, 1 reply; 7+ messages in thread
From: cinnamon shia @ 2018-05-11 4:17 UTC (permalink / raw)
To: edk2-devel; +Cc: lersek, star.zeng, cinnamon shia, Ansen Huang
If Fvb is a NULL, EFI_NOT_FOUND should be returned.
If the remaining size is not enough, EFI_OUT_OF_RESOURCES should be returned.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 7303681aaa..fc10cd9e18 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -238,6 +238,8 @@ IsValidVariableHeader (
@param Buffer Pointer to the buffer from which data is written.
@retval EFI_INVALID_PARAMETER Parameters not valid.
+ @retval EFI_NOT_FOUND Fvb is a NULL.
+ @retval EFI_OUT_OF_RESOURCES The remaining size is not enough.
@retval EFI_SUCCESS Variable store successfully updated.
**/
@@ -274,7 +276,7 @@ UpdateVariableStore (
//
if (!Volatile) {
if (Fvb == NULL) {
- return EFI_INVALID_PARAMETER;
+ return EFI_NOT_FOUND;
}
Status = Fvb->GetPhysicalAddress(Fvb, &FvVolHdr);
ASSERT_EFI_ERROR (Status);
@@ -289,7 +291,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
} else {
//
@@ -302,7 +304,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
//
--
2.16.1.windows.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] MdeModulePkg Variable: Failed to set a variable if the remaining size is equal to the variable data size.
2018-05-11 4:17 [PATCH 1/2] MdeModulePkg Variable: Failed to set a variable if the remaining size is equal to the variable data size cinnamon shia
2018-05-11 4:17 ` [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore() cinnamon shia
@ 2018-05-11 8:26 ` Zeng, Star
1 sibling, 0 replies; 7+ messages in thread
From: Zeng, Star @ 2018-05-11 8:26 UTC (permalink / raw)
To: cinnamon shia, edk2-devel@lists.01.org
Cc: lersek@redhat.com, Ansen Huang, Zeng, Star
Good catch and I agree the code change. Only one minor comment.
Please reduce the title length to <= 72 chars as I know it is requirement for patch check (PatchCheck.py in BaseTools) and push to server.
Thanks,
Star
-----Original Message-----
From: cinnamon shia [mailto:cinnamon.shia@hpe.com]
Sent: Friday, May 11, 2018 12:18 PM
To: edk2-devel@lists.01.org
Cc: lersek@redhat.com; Zeng, Star <star.zeng@intel.com>; cinnamon shia <cinnamon.shia@hpe.com>; Ansen Huang <ansen.huang@hpe.com>
Subject: [PATCH 1/2] MdeModulePkg Variable: Failed to set a variable if the remaining size is equal to the variable data size.
Fix the issue that failed to update or add a UEFI variable if the remaining size is equal to the data size
of the variable.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 6caf603b3d..7303681aaa 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -17,7 +17,7 @@
integer overflow. It should also check attribute to avoid authentication bypass.
Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
+(C) Copyright 2015-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
@@ -288,7 +288,7 @@ UpdateVariableStore (
DataPtr += mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase;
}
- if ((DataPtr + DataSize) >= ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
+ if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
return EFI_INVALID_PARAMETER;
}
} else {
@@ -301,7 +301,7 @@ UpdateVariableStore (
DataPtr += mVariableModuleGlobal->VariableGlobal.VolatileVariableBase;
}
- if ((DataPtr + DataSize) >= ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
+ if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
return EFI_INVALID_PARAMETER;
}
--
2.16.1.windows.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
2018-05-11 4:17 ` [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore() cinnamon shia
@ 2018-05-11 8:37 ` Zeng, Star
2018-05-11 8:43 ` Shia, Cinnamon
0 siblings, 1 reply; 7+ messages in thread
From: Zeng, Star @ 2018-05-11 8:37 UTC (permalink / raw)
To: cinnamon shia, edk2-devel@lists.01.org
Cc: lersek@redhat.com, Ansen Huang, Zeng, Star
Good patch. I have two minor comments.
1. Please reduce the title length to <= 72 chars as I know it is requirement for patch check (PatchCheck.py in BaseTools) and push to server.
2. I prefer to return EFI_UNSUPPORTED instead of EFI_NOT_FOUND for the case (Fvb == NULL).
@retval EFI_UNSUPPORTED Fvb is a NULL for Non-Volatile variable update.
Thanks,
Star
-----Original Message-----
From: cinnamon shia [mailto:cinnamon.shia@hpe.com]
Sent: Friday, May 11, 2018 12:18 PM
To: edk2-devel@lists.01.org
Cc: lersek@redhat.com; Zeng, Star <star.zeng@intel.com>; cinnamon shia <cinnamon.shia@hpe.com>; Ansen Huang <ansen.huang@hpe.com>
Subject: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
If Fvb is a NULL, EFI_NOT_FOUND should be returned.
If the remaining size is not enough, EFI_OUT_OF_RESOURCES should be returned.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 7303681aaa..fc10cd9e18 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -238,6 +238,8 @@ IsValidVariableHeader (
@param Buffer Pointer to the buffer from which data is written.
@retval EFI_INVALID_PARAMETER Parameters not valid.
+ @retval EFI_NOT_FOUND Fvb is a NULL.
+ @retval EFI_OUT_OF_RESOURCES The remaining size is not enough.
@retval EFI_SUCCESS Variable store successfully updated.
**/
@@ -274,7 +276,7 @@ UpdateVariableStore (
//
if (!Volatile) {
if (Fvb == NULL) {
- return EFI_INVALID_PARAMETER;
+ return EFI_NOT_FOUND;
}
Status = Fvb->GetPhysicalAddress(Fvb, &FvVolHdr);
ASSERT_EFI_ERROR (Status);
@@ -289,7 +291,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
} else {
//
@@ -302,7 +304,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
//
--
2.16.1.windows.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
2018-05-11 8:37 ` Zeng, Star
@ 2018-05-11 8:43 ` Shia, Cinnamon
2018-05-11 8:47 ` Zeng, Star
0 siblings, 1 reply; 7+ messages in thread
From: Shia, Cinnamon @ 2018-05-11 8:43 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: lersek@redhat.com, Huang, Ansen
Hi Star,
Thanks for your comments.
About returning EFI_NOT_FOUND for the case (Fvb == NULL), the idea is from GetFvbInfoByAddress().
Do you think we should apply the same logic to GetFvbInfoByAddress()?
Thanks,
Cinnamon Shia
-----Original Message-----
From: Zeng, Star [mailto:star.zeng@intel.com]
Sent: Friday, May 11, 2018 4:37 PM
To: Shia, Cinnamon <cinnamon.shia@hpe.com>; edk2-devel@lists.01.org
Cc: lersek@redhat.com; Huang, Ansen <ansen.huang@hpe.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
Good patch. I have two minor comments.
1. Please reduce the title length to <= 72 chars as I know it is requirement for patch check (PatchCheck.py in BaseTools) and push to server.
2. I prefer to return EFI_UNSUPPORTED instead of EFI_NOT_FOUND for the case (Fvb == NULL).
@retval EFI_UNSUPPORTED Fvb is a NULL for Non-Volatile variable update.
Thanks,
Star
-----Original Message-----
From: cinnamon shia [mailto:cinnamon.shia@hpe.com]
Sent: Friday, May 11, 2018 12:18 PM
To: edk2-devel@lists.01.org
Cc: lersek@redhat.com; Zeng, Star <star.zeng@intel.com>; cinnamon shia <cinnamon.shia@hpe.com>; Ansen Huang <ansen.huang@hpe.com>
Subject: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
If Fvb is a NULL, EFI_NOT_FOUND should be returned.
If the remaining size is not enough, EFI_OUT_OF_RESOURCES should be returned.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 7303681aaa..fc10cd9e18 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -238,6 +238,8 @@ IsValidVariableHeader (
@param Buffer Pointer to the buffer from which data is written.
@retval EFI_INVALID_PARAMETER Parameters not valid.
+ @retval EFI_NOT_FOUND Fvb is a NULL.
+ @retval EFI_OUT_OF_RESOURCES The remaining size is not enough.
@retval EFI_SUCCESS Variable store successfully updated.
**/
@@ -274,7 +276,7 @@ UpdateVariableStore (
//
if (!Volatile) {
if (Fvb == NULL) {
- return EFI_INVALID_PARAMETER;
+ return EFI_NOT_FOUND;
}
Status = Fvb->GetPhysicalAddress(Fvb, &FvVolHdr);
ASSERT_EFI_ERROR (Status);
@@ -289,7 +291,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
} else {
//
@@ -302,7 +304,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
//
--
2.16.1.windows.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
2018-05-11 8:43 ` Shia, Cinnamon
@ 2018-05-11 8:47 ` Zeng, Star
2018-05-11 8:49 ` Shia, Cinnamon
0 siblings, 1 reply; 7+ messages in thread
From: Zeng, Star @ 2018-05-11 8:47 UTC (permalink / raw)
To: Shia, Cinnamon, edk2-devel@lists.01.org
Cc: lersek@redhat.com, Huang, Ansen, Zeng, Star
My understanding is GetFvbInfoByAddress() is to *Get* something, then EFI_NOT_FOUND is better.
But UpdateVariableStore() is to *Update* something, I think EFI_UNSUPPORTED is better.
Thanks,
Star
-----Original Message-----
From: Shia, Cinnamon [mailto:cinnamon.shia@hpe.com]
Sent: Friday, May 11, 2018 4:43 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: lersek@redhat.com; Huang, Ansen <ansen.huang@hpe.com>
Subject: RE: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
Hi Star,
Thanks for your comments.
About returning EFI_NOT_FOUND for the case (Fvb == NULL), the idea is from GetFvbInfoByAddress().
Do you think we should apply the same logic to GetFvbInfoByAddress()?
Thanks,
Cinnamon Shia
-----Original Message-----
From: Zeng, Star [mailto:star.zeng@intel.com]
Sent: Friday, May 11, 2018 4:37 PM
To: Shia, Cinnamon <cinnamon.shia@hpe.com>; edk2-devel@lists.01.org
Cc: lersek@redhat.com; Huang, Ansen <ansen.huang@hpe.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
Good patch. I have two minor comments.
1. Please reduce the title length to <= 72 chars as I know it is requirement for patch check (PatchCheck.py in BaseTools) and push to server.
2. I prefer to return EFI_UNSUPPORTED instead of EFI_NOT_FOUND for the case (Fvb == NULL).
@retval EFI_UNSUPPORTED Fvb is a NULL for Non-Volatile variable update.
Thanks,
Star
-----Original Message-----
From: cinnamon shia [mailto:cinnamon.shia@hpe.com]
Sent: Friday, May 11, 2018 12:18 PM
To: edk2-devel@lists.01.org
Cc: lersek@redhat.com; Zeng, Star <star.zeng@intel.com>; cinnamon shia <cinnamon.shia@hpe.com>; Ansen Huang <ansen.huang@hpe.com>
Subject: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
If Fvb is a NULL, EFI_NOT_FOUND should be returned.
If the remaining size is not enough, EFI_OUT_OF_RESOURCES should be returned.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 7303681aaa..fc10cd9e18 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -238,6 +238,8 @@ IsValidVariableHeader (
@param Buffer Pointer to the buffer from which data is written.
@retval EFI_INVALID_PARAMETER Parameters not valid.
+ @retval EFI_NOT_FOUND Fvb is a NULL.
+ @retval EFI_OUT_OF_RESOURCES The remaining size is not enough.
@retval EFI_SUCCESS Variable store successfully updated.
**/
@@ -274,7 +276,7 @@ UpdateVariableStore (
//
if (!Volatile) {
if (Fvb == NULL) {
- return EFI_INVALID_PARAMETER;
+ return EFI_NOT_FOUND;
}
Status = Fvb->GetPhysicalAddress(Fvb, &FvVolHdr);
ASSERT_EFI_ERROR (Status);
@@ -289,7 +291,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
} else {
//
@@ -302,7 +304,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
//
--
2.16.1.windows.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
2018-05-11 8:47 ` Zeng, Star
@ 2018-05-11 8:49 ` Shia, Cinnamon
0 siblings, 0 replies; 7+ messages in thread
From: Shia, Cinnamon @ 2018-05-11 8:49 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: lersek@redhat.com, Huang, Ansen
Make sense to me. Will address your comments in the patch v2.
Thanks,
Cinnamon Shia
-----Original Message-----
From: Zeng, Star [mailto:star.zeng@intel.com]
Sent: Friday, May 11, 2018 4:47 PM
To: Shia, Cinnamon <cinnamon.shia@hpe.com>; edk2-devel@lists.01.org
Cc: lersek@redhat.com; Huang, Ansen <ansen.huang@hpe.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
My understanding is GetFvbInfoByAddress() is to *Get* something, then EFI_NOT_FOUND is better.
But UpdateVariableStore() is to *Update* something, I think EFI_UNSUPPORTED is better.
Thanks,
Star
-----Original Message-----
From: Shia, Cinnamon [mailto:cinnamon.shia@hpe.com]
Sent: Friday, May 11, 2018 4:43 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: lersek@redhat.com; Huang, Ansen <ansen.huang@hpe.com>
Subject: RE: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
Hi Star,
Thanks for your comments.
About returning EFI_NOT_FOUND for the case (Fvb == NULL), the idea is from GetFvbInfoByAddress().
Do you think we should apply the same logic to GetFvbInfoByAddress()?
Thanks,
Cinnamon Shia
-----Original Message-----
From: Zeng, Star [mailto:star.zeng@intel.com]
Sent: Friday, May 11, 2018 4:37 PM
To: Shia, Cinnamon <cinnamon.shia@hpe.com>; edk2-devel@lists.01.org
Cc: lersek@redhat.com; Huang, Ansen <ansen.huang@hpe.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
Good patch. I have two minor comments.
1. Please reduce the title length to <= 72 chars as I know it is requirement for patch check (PatchCheck.py in BaseTools) and push to server.
2. I prefer to return EFI_UNSUPPORTED instead of EFI_NOT_FOUND for the case (Fvb == NULL).
@retval EFI_UNSUPPORTED Fvb is a NULL for Non-Volatile variable update.
Thanks,
Star
-----Original Message-----
From: cinnamon shia [mailto:cinnamon.shia@hpe.com]
Sent: Friday, May 11, 2018 12:18 PM
To: edk2-devel@lists.01.org
Cc: lersek@redhat.com; Zeng, Star <star.zeng@intel.com>; cinnamon shia <cinnamon.shia@hpe.com>; Ansen Huang <ansen.huang@hpe.com>
Subject: [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore().
If Fvb is a NULL, EFI_NOT_FOUND should be returned.
If the remaining size is not enough, EFI_OUT_OF_RESOURCES should be returned.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: cinnamon shia <cinnamon.shia@hpe.com>
Signed-off-by: Ansen Huang <ansen.huang@hpe.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 7303681aaa..fc10cd9e18 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -238,6 +238,8 @@ IsValidVariableHeader (
@param Buffer Pointer to the buffer from which data is written.
@retval EFI_INVALID_PARAMETER Parameters not valid.
+ @retval EFI_NOT_FOUND Fvb is a NULL.
+ @retval EFI_OUT_OF_RESOURCES The remaining size is not enough.
@retval EFI_SUCCESS Variable store successfully updated.
**/
@@ -274,7 +276,7 @@ UpdateVariableStore (
//
if (!Volatile) {
if (Fvb == NULL) {
- return EFI_INVALID_PARAMETER;
+ return EFI_NOT_FOUND;
}
Status = Fvb->GetPhysicalAddress(Fvb, &FvVolHdr);
ASSERT_EFI_ERROR (Status);
@@ -289,7 +291,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((EFI_PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) FwVolHeader + FwVolHeader->FvLength))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
} else {
//
@@ -302,7 +304,7 @@ UpdateVariableStore (
}
if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
- return EFI_INVALID_PARAMETER;
+ return EFI_OUT_OF_RESOURCES;
}
//
--
2.16.1.windows.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-05-11 8:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-11 4:17 [PATCH 1/2] MdeModulePkg Variable: Failed to set a variable if the remaining size is equal to the variable data size cinnamon shia
2018-05-11 4:17 ` [PATCH 2/2] MdeModulePkg Variable: Correct the returned EFI_STATUS in the UpdateVariableStore() cinnamon shia
2018-05-11 8:37 ` Zeng, Star
2018-05-11 8:43 ` Shia, Cinnamon
2018-05-11 8:47 ` Zeng, Star
2018-05-11 8:49 ` Shia, Cinnamon
2018-05-11 8:26 ` [PATCH 1/2] MdeModulePkg Variable: Failed to set a variable if the remaining size is equal to the variable data size Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox