Re: [PATCH V2 06/15] MdeModulePkg Variable: Remove CacheOffset in UpdateVariable()

["Wang, Jian J" <jian.j.wang@intel.com>]

Star,

Just a tiny comment below. With it's addressed, 

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, January 14, 2019 11:20 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Wu, Hao A <hao.a.wu@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH V2 06/15] MdeModulePkg Variable: Remove CacheOffset in
> UpdateVariable()
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1323
> Merge EmuVariable and Real variable driver.
> 
> CacheOffset could be removed in UpdateVariable() after
> //
> // update the memory copy of Flash region.
> //
> CopyMem (
>   (UINT8 *)mNvVariableCache + CacheOffset,
>   (UINT8 *)NextVariable, VarSize
>    );
> 
> is moved to be before mVariableModuleGlobal->NonVolatileLastVariableOffset
> value is updated, like right before
> 
> mVariableModuleGlobal->NonVolatileLastVariableOffset +=
>   HEADER_ALIGN (VarSize);
> 
> This patch prepares for adding emulated variable NV mode
> support in VariableRuntimeDxe.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index 424f92a53757..4d524db30fec 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -2139,7 +2139,6 @@ UpdateVariable (
>    VARIABLE_POINTER_TRACK              *Variable;
>    VARIABLE_POINTER_TRACK              NvVariable;
>    VARIABLE_STORE_HEADER               *VariableStoreHeader;
> -  UINTN                               CacheOffset;
>    UINT8                               *BufferForMerge;
>    UINTN                               MergedBufSize;
>    BOOLEAN                             DataReady;
> @@ -2577,7 +2576,6 @@ UpdateVariable (
>      //
>      // Step 1:
>      //
> -    CacheOffset = mVariableModuleGlobal->NonVolatileLastVariableOffset;
>      Status = UpdateVariableStore (
>                 &mVariableModuleGlobal->VariableGlobal,
>                 FALSE,
> @@ -2643,6 +2641,11 @@ UpdateVariable (
>        goto Done;
>      }
> 
> +    //
> +    // update the memory copy of Flash region.
> +    //

The first character of comment is not capitalized.

> +    CopyMem ((UINT8 *)mNvVariableCache + mVariableModuleGlobal-
> >NonVolatileLastVariableOffset, (UINT8 *)NextVariable, VarSize);
> +
>      mVariableModuleGlobal->NonVolatileLastVariableOffset += HEADER_ALIGN
> (VarSize);
> 
>      if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) != 0) {
> @@ -2653,10 +2656,6 @@ UpdateVariable (
>          mVariableModuleGlobal->CommonUserVariableTotalSize +=
> HEADER_ALIGN (VarSize);
>        }
>      }
> -    //
> -    // update the memory copy of Flash region.
> -    //
> -    CopyMem ((UINT8 *)mNvVariableCache + CacheOffset, (UINT8
> *)NextVariable, VarSize);
>    } else {
>      //
>      // Create a volatile variable.
> --
> 2.7.0.windows.1