[edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
Guomin Jiang
guomin.jiang at intel.com
Wed Apr 1 02:37:03 UTC 2020
It is ok, I have no others confusion.
Reviewed-by: Guomin Jiang <guomin.jiang at intel.com>
> -----Original Message-----
> From: Michael Kubacki [mailto:michael.kubacki at outlook.com]
> Sent: Wednesday, April 1, 2020 1:12 AM
> To: Jiang, Guomin <guomin.jiang at intel.com>; devel at edk2.groups.io; Wang,
> Jian J <jian.j.wang at intel.com>
> Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>; Gao, Liming
> <liming.gao at intel.com>; Kinney, Michael D <michael.d.kinney at intel.com>;
> Wu, Hao A <hao.a.wu at intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return
> GetVariable() attr if EFI_BUFFER_TOO_SMALL
>
> That has been spelled incorrectly for about 9 years. The file (like many
> others) also has other spelling errors such as the following. I suggest this be
> fixed in a separate commit/series focused on fixing spelling errors.
>
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -102,7 +102,7 @@ AUTH_VAR_LIB_CONTEXT_OUT mAuthContextOut;
> This function writes data to the FWH at the correct LBA even if the LBAs
> are fragmented.
>
> - @param Global Pointer to VARAIBLE_GLOBAL structure.
> + @param Global Pointer to VARIABLE_GLOBAL structure.
> @param Volatile Point out the Variable is Volatile or
> Non-Volatile.
> @param SetByIndex TRUE if target pointer is given as index.
> FALSE if target pointer is absolute.
> @@ -504,7 +504,7 @@ InitializeVariableQuota (
>
> @return EFI_SUCCESS Reclaim operation has finished
> successfully.
> @return EFI_OUT_OF_RESOURCES No enough memory resources or
> variable space.
> - @return Others Unexpect error happened during
> reclaim operation.
> + @return Others Unexpected error happened during
> reclaim operation.
>
> **/
> EFI_STATUS
> @@ -2561,7 +2561,7 @@ VariableServiceSetVariable (
> }
>
> //
> - // Check for reserverd bit in variable attribute.
> + // Check for reserved bit in variable attribute.
> // EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated but we
> still allow
> // the delete operation of common authenticated variable at user physical
> presence.
> // So leave EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS attribute
> check to AuthVariableLib @@ -3381,7 +3381,7 @@
> ConvertNormalVarStorageToAuthVarStorage (
> VARIABLE_HEADER *StartPtr;
> UINT8 *NextPtr;
> VARIABLE_HEADER *EndPtr;
> - UINTN AuthVarStroageSize;
> + UINTN AuthVarStorageSize;
> AUTHENTICATED_VARIABLE_HEADER *AuthStartPtr;
> VARIABLE_STORE_HEADER *AuthVarStorage;
>
>
> On 3/31/2020 1:18 AM, Jiang, Guomin wrote:
> > There is a spell error in the comments of VariableServiceGetVariable() in
> Variable.c.
> > - @return EFI_BUFFER_TO_SMALL DataSize is too small for the result.
> > + @return EFI_BUFFER_TOO_SMALL DataSize is too small for the result.
> >
> > Need create new bugs for it or fix in this comment directly?
> >
> >> -----Original Message-----
> >> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
> >> Wang, Jian J
> >> Sent: Monday, March 30, 2020 12:16 PM
> >> To: michael.kubacki at outlook.com; devel at edk2.groups.io
> >> Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>; Gao, Liming
> >> <liming.gao at intel.com>; Kinney, Michael D
> >> <michael.d.kinney at intel.com>; Wu, Hao A <hao.a.wu at intel.com>
> >> Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable:
> >> Return
> >> GetVariable() attr if EFI_BUFFER_TOO_SMALL
> >>
> >>
> >> Reviewed-by: Jian J Wang <jian.j.wang at intel.com>
> >>
> >> Regards,
> >> Jian
> >>
> >>> -----Original Message-----
> >>> From: michael.kubacki at outlook.com <michael.kubacki at outlook.com>
> >>> Sent: Saturday, March 28, 2020 5:56 AM
> >>> To: devel at edk2.groups.io
> >>> Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>; Gao, Liming
> >>> <liming.gao at intel.com>; Kinney, Michael D
> >>> <michael.d.kinney at intel.com>; Wang, Jian J <jian.j.wang at intel.com>;
> >>> Wu, Hao A <hao.a.wu at intel.com>
> >>> Subject: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable()
> >>> attr if EFI_BUFFER_TOO_SMALL
> >>>
> >>> From: Michael Kubacki <michael.kubacki at microsoft.com>
> >>>
> >>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
> >>>
> >>> The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
> >>> "Attributes" parameter description states:
> >>>
> >>> "If not NULL, a pointer to the memory location to return the
> >>> attributes bitmask for the variable. See 'Related Definitions.'
> >>> If not NULL, then Attributes is set on output both when
> >>> EFI_SUCCESS and when EFI_BUFFER_TOO_SMALL is returned."
> >>>
> >>> The attributes were previously only returned from the implementation
> >>> in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS
> >>> or EFI_BUFFER_TOO_SMALL according to spec.
> >>>
> >>> Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>
> >>> Cc: Liming Gao <liming.gao at intel.com>
> >>> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> >>> Cc: Jian J Wang <jian.j.wang at intel.com>
> >>> Cc: Hao A Wu <hao.a.wu at intel.com>
> >>> Signed-off-by: Michael Kubacki <michael.kubacki at microsoft.com>
> >>> ---
> >>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10
> >>> +++++++---
> >>>
> >>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> >> c |
> >>> 10 ++++++----
> >>> 2 files changed, 13 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>> index d23aea4bc712..1e71fc642c76 100644
> >>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>> @@ -18,6 +18,8 @@
> >>>
> >>> Copyright (c) 2006 - 2020, Intel Corporation. All rights
> >>> reserved.<BR>
> >>> (C) Copyright 2015-2018 Hewlett Packard Enterprise Development
> >>> LP<BR>
> >>> +Copyright (c) Microsoft Corporation.<BR>
> >>> +
> >>> SPDX-License-Identifier: BSD-2-Clause-Patent
> >>>
> >>> **/
> >>> @@ -2379,9 +2381,6 @@ VariableServiceGetVariable (
> >>> }
> >>>
> >>> CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr,
> >>> mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize);
> >>> - if (Attributes != NULL) {
> >>> - *Attributes = Variable.CurrPtr->Attributes;
> >>> - }
> >>>
> >>> *DataSize = VarDataSize;
> >>> UpdateVariableInfo (VariableName, VendorGuid,
> >>> Variable.Volatile, TRUE, FALSE, FALSE, FALSE, &gVariableInfo); @@
> >>> -2395,6 +2394,11 @@ VariableServiceGetVariable (
> >>> }
> >>>
> >>> Done:
> >>> + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> >>> + if (Attributes != NULL && Variable.CurrPtr != NULL) {
> >>> + *Attributes = Variable.CurrPtr->Attributes;
> >>> + }
> >>> + }
> >>> ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal-
> >>>> VariableGlobal.VariableServicesLock);
> >>> return Status;
> >>> }
> >>> diff --git
> >>>
> >>
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> >> e.c
> >>>
> >>
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> >> e.c
> >>> index 2cf0ed32ae55..ca833fb0244d 100644
> >>> ---
> >>>
> >>
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> >> e.c
> >>> +++
> >>>
> >>
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> >> e.c
> >>> @@ -14,6 +14,7 @@
> >>> InitCommunicateBuffer() is really function to check the variable data
> size.
> >>>
> >>> Copyright (c) 2010 - 2019, Intel Corporation. All rights
> >>> reserved.<BR>
> >>> +Copyright (c) Microsoft Corporation.<BR>
> >>> SPDX-License-Identifier: BSD-2-Clause-Patent
> >>>
> >>> **/
> >>> @@ -642,10 +643,6 @@ FindVariableInRuntimeCache (
> >>> }
> >>>
> >>> CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr,
> >>> mVariableAuthFormat), TempDataSize);
> >>> - if (Attributes != NULL) {
> >>> - *Attributes = RtPtrTrack.CurrPtr->Attributes;
> >>> - }
> >>> -
> >>> *DataSize = TempDataSize;
> >>>
> >>> UpdateVariableInfo (VariableName, VendorGuid,
> >>> RtPtrTrack.Volatile, TRUE, FALSE, FALSE, TRUE, &mVariableInfo); @@
> >>> -661,6 +658,11 @@ FindVariableInRuntimeCache (
> >>> }
> >>>
> >>> Done:
> >>> + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> >>> + if (Attributes != NULL && RtPtrTrack.CurrPtr != NULL) {
> >>> + *Attributes = RtPtrTrack.CurrPtr->Attributes;
> >>> + }
> >>> + }
> >>> mVariableRuntimeCacheReadLock = FALSE;
> >>>
> >>> return Status;
> >>> --
> >>> 2.16.3.windows.1
> >>
> >>
> >>
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56809): https://edk2.groups.io/g/devel/message/56809
Mute This Topic: https://groups.io/mt/72598884/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list