* Re: [PATCH] ShellPkg/dmpstore: Support "-sfo"
2016-11-11 10:14 [PATCH] ShellPkg/dmpstore: Support "-sfo" Ruiyu Ni
@ 2016-11-11 14:20 ` Carsey, Jaben
2016-11-11 17:21 ` Shah, Tapan
2016-11-11 20:43 ` Shah, Tapan
2 siblings, 0 replies; 6+ messages in thread
From: Carsey, Jaben @ 2016-11-11 14:20 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Cc: Chen, Chen A, Tapan Shah, Carsey, Jaben
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Friday, November 11, 2016 2:14 AM
> To: edk2-devel@lists.01.org
> Cc: Chen, Chen A <chen.a.chen@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; Tapan Shah <tapandshah@hpe.com>
> Subject: [PATCH] ShellPkg/dmpstore: Support "-sfo"
> Importance: High
>
> The patch adds the "-sfo" support to "dmpstore" command.
>
> 1. For "-l" (load variable from file), when the variable
> content can be updated, the new variable data is displayed
> in SFO format, otherwise, the new variable data is not
> displayed. So that the SFO consumer can know which variables
> are updated by parsing the SFO.
>
> 2. For "-d" (delete variable), when the variable can be deleted
> successfully, only the variable name and GUID are displayed in
> SFO but the attribute/data/data size are displayed as empty to
> indicate the deletion happened; otherwise, the variable is not
> displayed.
>
> 3. For displaying variable, when the variable specified by name
> and GUID cannot be found, an error message is displayed; Otherwise,
> the SFO is displayed.
> E.g.: "dmpstore -guid GuidThatDoesntExist -sfo" produces output
> as:
> ShellCommand,"dmpstore"
> VariableInfo,"","GuidThatDoesntExist","","",""
>
> "dmpstore NameThatDoesntExist -guid GuidThatDoesntExist -sfo"
> produces output as:
> ShellCommand,"dmpstore"
> dmpstore: No matching variables found. Guid GuidThatDoesntExist, Name
> NameThatDoesntExist
>
> The difference between the above 2 cases is that former one only
> specifies the GUID, but the latter one specifies both name and GUID.
> Since not specifying GUID means to use GlobalVariableGuid,
> "dmpstore NameThatDoesntExist -sfo" produces the similar output as
> latter one.
> I personally prefer to always produce SFO output for both cases.
> But the above behavior is the discussion result between HPE engineers.
>
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Tapan Shah <tapandshah@hpe.com>
> ---
> .../Library/UefiShellDebug1CommandsLib/DmpStore.c | 269
> +++++++++++++++------
> .../UefiShellDebug1CommandsLib.uni | 5 +
> 2 files changed, 202 insertions(+), 72 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> index 3427c99..aa8ad09 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> @@ -2,7 +2,7 @@
> Main file for DmpStore shell Debug1 function.
>
> (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.<BR>
> - Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<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
> @@ -82,12 +82,46 @@ GetAttrType (
> }
>
> /**
> + Convert binary to hex format string.
> +
> + @param[in] BufferSize The size in bytes of the binary data.
> + @param[in] Buffer The binary data.
> + @param[in, out] HexString Hex format string.
> + @param[in] HexStringSize The size in bytes of the string.
> +
> + @return The hex format string.
> +**/
> +CHAR16*
> +BinaryToHexString (
> + IN VOID *Buffer,
> + IN UINTN BufferSize,
> + IN OUT CHAR16 *HexString,
> + IN UINTN HexStringSize
> + )
> +{
> + UINTN Index;
> + UINTN StringIndex;
> +
> + for (Index = 0, StringIndex = 0; Index < BufferSize; Index += 1) {
> + StringIndex +=
> + UnicodeSPrint (
> + &HexString[StringIndex],
> + HexStringSize - StringIndex * sizeof (CHAR16),
> + L"%02x",
> + ((UINT8 *) Buffer)[Index]
> + );
> + }
> + return HexString;
> +}
> +
> +/**
> Load the variable data from file and set to variable data base.
>
> - @param[in] FileHandle The file to be read.
> - @param[in] Name The name of the variables to be loaded.
> - @param[in] Guid The guid of the variables to be loaded.
> - @param[out] Found TRUE when at least one variable was loaded and
> set.
> + @param[in] FileHandle The file to be read.
> + @param[in] Name The name of the variables to be loaded.
> + @param[in] Guid The guid of the variables to be loaded.
> + @param[out] Found TRUE when at least one variable was loaded
> and set.
> + @param[in] StandardFormatOutput TRUE indicates Standard-Format
> Output.
>
> @retval SHELL_DEVICE_ERROR Cannot access the file.
> @retval SHELL_VOLUME_CORRUPTED The file is in bad format.
> @@ -99,7 +133,8 @@ LoadVariablesFromFile (
> IN SHELL_FILE_HANDLE FileHandle,
> IN CONST CHAR16 *Name,
> IN CONST EFI_GUID *Guid,
> - OUT BOOLEAN *Found
> + OUT BOOLEAN *Found,
> + IN BOOLEAN StandardFormatOutput
> )
> {
> EFI_STATUS Status;
> @@ -116,6 +151,7 @@ LoadVariablesFromFile (
> CHAR16 *Attributes;
> UINT8 *Buffer;
> UINT32 Crc32;
> + CHAR16 *HexString;
>
> Status = ShellGetFileSize (FileHandle, &FileSize);
> if (EFI_ERROR (Status)) {
> @@ -221,11 +257,6 @@ LoadVariablesFromFile (
> ((Guid == NULL) || CompareGuid (&Variable->Guid, Guid))
> ) {
> Attributes = GetAttrType (Variable->Attributes);
> - ShellPrintHiiEx (
> - -1, -1, NULL, STRING_TOKEN(STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> - Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
> - );
> - SHELL_FREE_NON_NULL(Attributes);
>
> *Found = TRUE;
> Status = gRT->SetVariable (
> @@ -236,8 +267,38 @@ LoadVariablesFromFile (
> Variable->Data
> );
> if (EFI_ERROR (Status)) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_LOAD_GEN_FAIL), gShellDebug1HiiHandle, L"dmpstore",
> Variable->Name, Status);
> + if (StandardFormatOutput) {
> + //
> + // Supress SFO to indicate the loading is failed.
> + //
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD_GEN_FAIL),
> gShellDebug1HiiHandle,
> + L"dmpstore", Variable->Name, Status
> + );
> + }
> + } else {
> + if (StandardFormatOutput) {
> + HexString = AllocatePool ((Variable->DataSize * 2 + 1) * sizeof
> (CHAR16));
> + if (HexString != NULL) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO),
> gShellDebug1HiiHandle,
> + Variable->Name, &Variable->Guid, Variable->Attributes, Variable-
> >DataSize,
> + BinaryToHexString (
> + Variable->Data, Variable->DataSize, HexString,
> + (Variable->DataSize * 2 + 1) * sizeof (CHAR16)
> + )
> + );
> + FreePool (HexString);
> + }
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> + Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
> + );
> + }
> }
> + SHELL_FREE_NON_NULL(Attributes);
> }
> }
>
> @@ -350,14 +411,15 @@ AppendSingleVariableToFile (
>
> This is necessary since once a delete happens GetNextVariableName() will
> work.
>
> - @param[in] Name The variable name of the EFI variable (or NULL).
> - @param[in] Guid The GUID of the variable set (or NULL).
> - @param[in] Type The operation type.
> - @param[in] FileHandle The file to operate on (or NULL).
> - @param[in] PrevName The previous variable name from
> GetNextVariableName. L"" to start.
> - @param[in] FoundVarGuid The previous GUID from
> GetNextVariableName. ignored at start.
> - @param[in] FoundOne If a VariableName or Guid was specified and one
> was printed or
> - deleted, then set this to TRUE, otherwise ignored.
> + @param[in] Name The variable name of the EFI variable (or NULL).
> + @param[in] Guid The GUID of the variable set (or NULL).
> + @param[in] Type The operation type.
> + @param[in] FileHandle The file to operate on (or NULL).
> + @param[in] PrevName The previous variable name from
> GetNextVariableName. L"" to start.
> + @param[in] FoundVarGuid The previous GUID from
> GetNextVariableName. ignored at start.
> + @param[in] FoundOne If a VariableName or Guid was specified and
> one was printed or
> + deleted, then set this to TRUE, otherwise ignored.
> + @param[in] StandardFormatOutput TRUE indicates Standard-Format
> Output.
>
> @retval SHELL_SUCCESS The operation was successful.
> @retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
> @@ -373,7 +435,8 @@ CascadeProcessVariables (
> IN EFI_FILE_PROTOCOL *FileHandle OPTIONAL,
> IN CONST CHAR16 * CONST PrevName,
> IN EFI_GUID FoundVarGuid,
> - IN BOOLEAN *FoundOne
> + IN BOOLEAN *FoundOne,
> + IN BOOLEAN StandardFormatOutput
> )
> {
> EFI_STATUS Status;
> @@ -383,7 +446,8 @@ CascadeProcessVariables (
> UINT32 Atts;
> SHELL_STATUS ShellStatus;
> UINTN NameSize;
> - CHAR16 *RetString;
> + CHAR16 *AttrString;
> + CHAR16 *HexString;
>
> if (ShellGetExecutionBreakFlag()) {
> return (SHELL_ABORTED);
> @@ -427,7 +491,7 @@ CascadeProcessVariables (
> //
> // Recurse to the next iteration. We know "our" variable's name.
> //
> - ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle,
> FoundVarName, FoundVarGuid, FoundOne);
> + ShellStatus = CascadeProcessVariables (Name, Guid, Type, FileHandle,
> FoundVarName, FoundVarGuid, FoundOne, StandardFormatOutput);
>
> if (ShellGetExecutionBreakFlag() || (ShellStatus == SHELL_ABORTED)) {
> SHELL_FREE_NON_NULL(FoundVarName);
> @@ -459,50 +523,86 @@ CascadeProcessVariables (
> Status = gRT->GetVariable (FoundVarName, &FoundVarGuid, &Atts,
> &DataSize, DataBuffer);
> }
> }
> - if ((Type == DmpStoreDisplay) || (Type == DmpStoreSave)) {
> //
> // Last error check then print this variable out.
> //
> + if (Type == DmpStoreDisplay) {
> + if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName !=
> NULL)) {
> + AttrString = GetAttrType(Atts);
> + if (StandardFormatOutput) {
> + HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
> + if (HexString != NULL) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO),
> gShellDebug1HiiHandle,
> + FoundVarName, &FoundVarGuid, Atts, DataSize,
> + BinaryToHexString (
> + DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
> + )
> + );
> + FreePool (HexString);
> + } else {
> + Status = EFI_OUT_OF_RESOURCES;
> + }
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> + AttrString, &FoundVarGuid, FoundVarName, DataSize
> + );
> + DumpHex (2, 0, DataSize, DataBuffer);
> + }
> + SHELL_FREE_NON_NULL (AttrString);
> + }
> + } else if (Type == DmpStoreSave) {
> if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName !=
> NULL)) {
> - RetString = GetAttrType(Atts);
> - ShellPrintHiiEx(
> - -1,
> - -1,
> - NULL,
> - STRING_TOKEN(STR_DMPSTORE_HEADER_LINE),
> - gShellDebug1HiiHandle,
> - RetString,
> - &FoundVarGuid,
> - FoundVarName,
> - DataSize);
> - if (Type == DmpStoreDisplay) {
> - DumpHex(2, 0, DataSize, DataBuffer);
> + AttrString = GetAttrType (Atts);
> + if (StandardFormatOutput) {
> + HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
> + if (HexString != NULL) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO),
> gShellDebug1HiiHandle,
> + FoundVarName, &FoundVarGuid, Atts, DataSize,
> + BinaryToHexString (
> + DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
> + )
> + );
> + FreePool (HexString);
> + } else {
> + Status = EFI_OUT_OF_RESOURCES;
> + }
> } else {
> - Status = AppendSingleVariableToFile (
> - FileHandle,
> - FoundVarName,
> - &FoundVarGuid,
> - Atts,
> - (UINT32) DataSize,
> - DataBuffer
> - );
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> + AttrString, &FoundVarGuid, FoundVarName, DataSize
> + );
> }
> - SHELL_FREE_NON_NULL(RetString);
> + Status = AppendSingleVariableToFile (
> + FileHandle,
> + FoundVarName,
> + &FoundVarGuid,
> + Atts,
> + (UINT32) DataSize,
> + DataBuffer
> + );
> + SHELL_FREE_NON_NULL (AttrString);
> }
> } else if (Type == DmpStoreDelete) {
> //
> // We only need name to delete it...
> //
> - ShellPrintHiiEx (
> - -1,
> - -1,
> - NULL,
> - STRING_TOKEN(STR_DMPSTORE_DELETE_LINE),
> - gShellDebug1HiiHandle,
> - &FoundVarGuid,
> - FoundVarName,
> - gRT->SetVariable (FoundVarName, &FoundVarGuid, Atts, 0, NULL)
> - );
> + EFI_STATUS SetStatus = gRT->SetVariable (FoundVarName,
> &FoundVarGuid, Atts, 0, NULL);
> + if (StandardFormatOutput) {
> + if (SetStatus == EFI_SUCCESS) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_NG_SFO), gShellDebug1HiiHandle,
> + FoundVarName, &FoundVarGuid
> + );
> + }
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_DELETE_LINE),
> gShellDebug1HiiHandle,
> + &FoundVarGuid, FoundVarName, SetStatus
> + );
> + }
> }
> SHELL_FREE_NON_NULL(DataBuffer);
> }
> @@ -523,10 +623,11 @@ CascadeProcessVariables (
> /**
> Function to display or delete variables. This will set up and call into the
> recursive function.
>
> - @param[in] Name The variable name of the EFI variable (or NULL).
> - @param[in] Guid The GUID of the variable set (or NULL).
> - @param[in] Type The operation type.
> - @param[in] FileHandle The file to save or load variables.
> + @param[in] Name The variable name of the EFI variable (or NULL).
> + @param[in] Guid The GUID of the variable set (or NULL).
> + @param[in] Type The operation type.
> + @param[in] FileHandle The file to save or load variables.
> + @param[in] StandardFormatOutput TRUE indicates Standard-Format
> Output.
>
> @retval SHELL_SUCCESS The operation was successful.
> @retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
> @@ -539,7 +640,8 @@ ProcessVariables (
> IN CONST CHAR16 *Name OPTIONAL,
> IN CONST EFI_GUID *Guid OPTIONAL,
> IN DMP_STORE_TYPE Type,
> - IN SHELL_FILE_HANDLE FileHandle OPTIONAL
> + IN SHELL_FILE_HANDLE FileHandle OPTIONAL,
> + IN BOOLEAN StandardFormatOutput
> )
> {
> SHELL_STATUS ShellStatus;
> @@ -550,10 +652,14 @@ ProcessVariables (
> ShellStatus = SHELL_SUCCESS;
> ZeroMem (&FoundVarGuid, sizeof(EFI_GUID));
>
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN(STR_GEN_SFO_HEADER),
> gShellDebug1HiiHandle, L"dmpstore");
> + }
> +
> if (Type == DmpStoreLoad) {
> - ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid, &Found);
> + ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid, &Found,
> StandardFormatOutput);
> } else {
> - ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle,
> NULL, FoundVarGuid, &Found);
> + ShellStatus = CascadeProcessVariables (Name, Guid, Type, FileHandle,
> NULL, FoundVarGuid, &Found, StandardFormatOutput);
> }
>
> if (!Found) {
> @@ -561,13 +667,25 @@ ProcessVariables (
> ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_OUT_MEM),
> gShellDebug1HiiHandle, L"dmpstore");
> return (ShellStatus);
> } else if (Name != NULL && Guid == NULL) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle,
> L"dmpstore", Name);
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_N_SFO), gShellDebug1HiiHandle,
> Name);
> + } else {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle,
> L"dmpstore", Name);
> + }
> } else if (Name != NULL && Guid != NULL) {
> ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_GN), gShellDebug1HiiHandle,
> L"dmpstore", Guid, Name);
> } else if (Name == NULL && Guid == NULL) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_SFO), gShellDebug1HiiHandle);
> + } else {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
> + }
> } else if (Name == NULL && Guid != NULL) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle,
> L"dmpstore", Guid);
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_G_SFO), gShellDebug1HiiHandle, Guid);
> + } else {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle,
> L"dmpstore", Guid);
> + }
> }
> return (SHELL_NOT_FOUND);
> }
> @@ -580,6 +698,7 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
> {L"-s", TypeValue},
> {L"-all", TypeFlag},
> {L"-guid", TypeValue},
> + {L"-sfo", TypeFlag},
> {NULL, TypeMax}
> };
>
> @@ -608,12 +727,14 @@ ShellCommandRunDmpStore (
> DMP_STORE_TYPE Type;
> SHELL_FILE_HANDLE FileHandle;
> EFI_FILE_INFO *FileInfo;
> + BOOLEAN StandardFormatOutput;
>
> - ShellStatus = SHELL_SUCCESS;
> - Package = NULL;
> - FileHandle = NULL;
> - File = NULL;
> - Type = DmpStoreDisplay;
> + ShellStatus = SHELL_SUCCESS;
> + Package = NULL;
> + FileHandle = NULL;
> + File = NULL;
> + Type = DmpStoreDisplay;
> + StandardFormatOutput = FALSE;
>
> Status = ShellCommandLineParse (ParamList, &Package, &ProblemParam,
> TRUE);
> if (EFI_ERROR(Status)) {
> @@ -742,6 +863,10 @@ ShellCommandRunDmpStore (
> } else if (ShellCommandLineGetFlag(Package, L"-d")) {
> Type = DmpStoreDelete;
> }
> +
> + if (ShellCommandLineGetFlag (Package,L"-sfo")) {
> + StandardFormatOutput = TRUE;
> + }
> }
>
> if (ShellStatus == SHELL_SUCCESS) {
> @@ -750,7 +875,7 @@ ShellCommandRunDmpStore (
> } else if (Type == DmpStoreLoad) {
> ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD),
> gShellDebug1HiiHandle, File);
> }
> - ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle);
> + ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle,
> StandardFormatOutput);
> if ((Type == DmpStoreLoad) || (Type == DmpStoreSave)) {
> ShellCloseFile (&FileHandle);
> }
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> index 52c2af0..c97bd62 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> @@ -407,9 +407,14 @@
> #string STR_DMPSTORE_HEADER_LINE #language en-US "Variable
> %H%s%N '%H%g%N:%H%s%N' DataSize = 0x%02x\r\n"
> #string STR_DMPSTORE_DELETE_LINE #language en-US "Delete variable
> '%H%g%N:%H%s%N': %r\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND #language en-US "%H%s%N:
> No matching variables found.\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_SFO #language en-US
> "VariableInfo,\"\",\"\",\"\",\"\",\"\"\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND_GN #language en-US
> "%H%s%N: No matching variables found. Guid %g, Name %s\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_NG_SFO #language en-US
> "VariableInfo,\"%s\",\"%g\",\"\",\"\",\"\"\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND_N #language en-US "%H%s%N:
> No matching variables found. Name %s\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_N_SFO #language en-US
> #language en-US "VariableInfo,\"%s\",\"\",\"\",\"\",\"\"\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND_G #language en-US "%H%s%N:
> No matching variables found. Guid %g\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_G_SFO #language en-US
> "VariableInfo,\"\",\"%g\",\"\",\"\",\"\"\r\n"
> +#string STR_DMPSTORE_VAR_SFO #language en-US
> "VariableInfo,\"%s\",\"%g\",\"0x%x\",\"0x%x\",\"%s\"\r\n"
>
> #string STR_GET_HELP_COMP #language en-US ""
> ".TH comp 0 "Compare 2 files"\r\n"
> --
> 2.9.0.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ShellPkg/dmpstore: Support "-sfo"
2016-11-11 10:14 [PATCH] ShellPkg/dmpstore: Support "-sfo" Ruiyu Ni
2016-11-11 14:20 ` Carsey, Jaben
@ 2016-11-11 17:21 ` Shah, Tapan
2016-11-14 2:24 ` Ni, Ruiyu
2016-11-11 20:43 ` Shah, Tapan
2 siblings, 1 reply; 6+ messages in thread
From: Shah, Tapan @ 2016-11-11 17:21 UTC (permalink / raw)
To: Ruiyu Ni, edk2-devel@lists.01.org; +Cc: Chen A Chen, Jaben Carsey
Two comments:
1. Missing help output update for -sfo support in dmpstore command.
2. BinaryToHexString() is missing input parameter NULL and out of bound size check before access.
-----Original Message-----
From: Ruiyu Ni [mailto:ruiyu.ni@intel.com]
Sent: Friday, November 11, 2016 4:14 AM
To: edk2-devel@lists.01.org
Cc: Chen A Chen <chen.a.chen@intel.com>; Jaben Carsey <jaben.carsey@intel.com>; Shah, Tapan <tapandshah@hpe.com>
Subject: [PATCH] ShellPkg/dmpstore: Support "-sfo"
The patch adds the "-sfo" support to "dmpstore" command.
1. For "-l" (load variable from file), when the variable
content can be updated, the new variable data is displayed
in SFO format, otherwise, the new variable data is not
displayed. So that the SFO consumer can know which variables
are updated by parsing the SFO.
2. For "-d" (delete variable), when the variable can be deleted
successfully, only the variable name and GUID are displayed in
SFO but the attribute/data/data size are displayed as empty to
indicate the deletion happened; otherwise, the variable is not
displayed.
3. For displaying variable, when the variable specified by name
and GUID cannot be found, an error message is displayed; Otherwise,
the SFO is displayed.
E.g.: "dmpstore -guid GuidThatDoesntExist -sfo" produces output
as:
ShellCommand,"dmpstore"
VariableInfo,"","GuidThatDoesntExist","","",""
"dmpstore NameThatDoesntExist -guid GuidThatDoesntExist -sfo"
produces output as:
ShellCommand,"dmpstore"
dmpstore: No matching variables found. Guid GuidThatDoesntExist, Name
NameThatDoesntExist
The difference between the above 2 cases is that former one only
specifies the GUID, but the latter one specifies both name and GUID.
Since not specifying GUID means to use GlobalVariableGuid,
"dmpstore NameThatDoesntExist -sfo" produces the similar output as
latter one.
I personally prefer to always produce SFO output for both cases.
But the above behavior is the discussion result between HPE engineers.
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Tapan Shah <tapandshah@hpe.com>
---
.../Library/UefiShellDebug1CommandsLib/DmpStore.c | 269 +++++++++++++++------
.../UefiShellDebug1CommandsLib.uni | 5 +
2 files changed, 202 insertions(+), 72 deletions(-)
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
index 3427c99..aa8ad09 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
@@ -2,7 +2,7 @@
Main file for DmpStore shell Debug1 function.
(C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.<BR>
- Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<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
@@ -82,12 +82,46 @@ GetAttrType (
}
/**
+ Convert binary to hex format string.
+
+ @param[in] BufferSize The size in bytes of the binary data.
+ @param[in] Buffer The binary data.
+ @param[in, out] HexString Hex format string.
+ @param[in] HexStringSize The size in bytes of the string.
+
+ @return The hex format string.
+**/
+CHAR16*
+BinaryToHexString (
+ IN VOID *Buffer,
+ IN UINTN BufferSize,
+ IN OUT CHAR16 *HexString,
+ IN UINTN HexStringSize
+ )
+{
+ UINTN Index;
+ UINTN StringIndex;
+
+ for (Index = 0, StringIndex = 0; Index < BufferSize; Index += 1) {
+ StringIndex +=
+ UnicodeSPrint (
+ &HexString[StringIndex],
+ HexStringSize - StringIndex * sizeof (CHAR16),
+ L"%02x",
+ ((UINT8 *) Buffer)[Index]
+ );
+ }
+ return HexString;
+}
+
+/**
Load the variable data from file and set to variable data base.
- @param[in] FileHandle The file to be read.
- @param[in] Name The name of the variables to be loaded.
- @param[in] Guid The guid of the variables to be loaded.
- @param[out] Found TRUE when at least one variable was loaded and set.
+ @param[in] FileHandle The file to be read.
+ @param[in] Name The name of the variables to be loaded.
+ @param[in] Guid The guid of the variables to be loaded.
+ @param[out] Found TRUE when at least one variable was loaded and set.
+ @param[in] StandardFormatOutput TRUE indicates Standard-Format Output.
@retval SHELL_DEVICE_ERROR Cannot access the file.
@retval SHELL_VOLUME_CORRUPTED The file is in bad format.
@@ -99,7 +133,8 @@ LoadVariablesFromFile (
IN SHELL_FILE_HANDLE FileHandle,
IN CONST CHAR16 *Name,
IN CONST EFI_GUID *Guid,
- OUT BOOLEAN *Found
+ OUT BOOLEAN *Found,
+ IN BOOLEAN StandardFormatOutput
)
{
EFI_STATUS Status;
@@ -116,6 +151,7 @@ LoadVariablesFromFile (
CHAR16 *Attributes;
UINT8 *Buffer;
UINT32 Crc32;
+ CHAR16 *HexString;
Status = ShellGetFileSize (FileHandle, &FileSize);
if (EFI_ERROR (Status)) {
@@ -221,11 +257,6 @@ LoadVariablesFromFile (
((Guid == NULL) || CompareGuid (&Variable->Guid, Guid))
) {
Attributes = GetAttrType (Variable->Attributes);
- ShellPrintHiiEx (
- -1, -1, NULL, STRING_TOKEN(STR_DMPSTORE_HEADER_LINE), gShellDebug1HiiHandle,
- Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
- );
- SHELL_FREE_NON_NULL(Attributes);
*Found = TRUE;
Status = gRT->SetVariable (
@@ -236,8 +267,38 @@ LoadVariablesFromFile (
Variable->Data
);
if (EFI_ERROR (Status)) {
- ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD_GEN_FAIL), gShellDebug1HiiHandle, L"dmpstore", Variable->Name, Status);
+ if (StandardFormatOutput) {
+ //
+ // Supress SFO to indicate the loading is failed.
+ //
+ } else {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD_GEN_FAIL), gShellDebug1HiiHandle,
+ L"dmpstore", Variable->Name, Status
+ );
+ }
+ } else {
+ if (StandardFormatOutput) {
+ HexString = AllocatePool ((Variable->DataSize * 2 + 1) * sizeof (CHAR16));
+ if (HexString != NULL) {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO), gShellDebug1HiiHandle,
+ Variable->Name, &Variable->Guid, Variable->Attributes, Variable->DataSize,
+ BinaryToHexString (
+ Variable->Data, Variable->DataSize, HexString,
+ (Variable->DataSize * 2 + 1) * sizeof (CHAR16)
+ )
+ );
+ FreePool (HexString);
+ }
+ } else {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE), gShellDebug1HiiHandle,
+ Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
+ );
+ }
}
+ SHELL_FREE_NON_NULL(Attributes);
}
}
@@ -350,14 +411,15 @@ AppendSingleVariableToFile (
This is necessary since once a delete happens GetNextVariableName() will work.
- @param[in] Name The variable name of the EFI variable (or NULL).
- @param[in] Guid The GUID of the variable set (or NULL).
- @param[in] Type The operation type.
- @param[in] FileHandle The file to operate on (or NULL).
- @param[in] PrevName The previous variable name from GetNextVariableName. L"" to start.
- @param[in] FoundVarGuid The previous GUID from GetNextVariableName. ignored at start.
- @param[in] FoundOne If a VariableName or Guid was specified and one was printed or
- deleted, then set this to TRUE, otherwise ignored.
+ @param[in] Name The variable name of the EFI variable (or NULL).
+ @param[in] Guid The GUID of the variable set (or NULL).
+ @param[in] Type The operation type.
+ @param[in] FileHandle The file to operate on (or NULL).
+ @param[in] PrevName The previous variable name from GetNextVariableName. L"" to start.
+ @param[in] FoundVarGuid The previous GUID from GetNextVariableName. ignored at start.
+ @param[in] FoundOne If a VariableName or Guid was specified and one was printed or
+ deleted, then set this to TRUE, otherwise ignored.
+ @param[in] StandardFormatOutput TRUE indicates Standard-Format Output.
@retval SHELL_SUCCESS The operation was successful.
@retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
@@ -373,7 +435,8 @@ CascadeProcessVariables (
IN EFI_FILE_PROTOCOL *FileHandle OPTIONAL,
IN CONST CHAR16 * CONST PrevName,
IN EFI_GUID FoundVarGuid,
- IN BOOLEAN *FoundOne
+ IN BOOLEAN *FoundOne,
+ IN BOOLEAN StandardFormatOutput
)
{
EFI_STATUS Status;
@@ -383,7 +446,8 @@ CascadeProcessVariables (
UINT32 Atts;
SHELL_STATUS ShellStatus;
UINTN NameSize;
- CHAR16 *RetString;
+ CHAR16 *AttrString;
+ CHAR16 *HexString;
if (ShellGetExecutionBreakFlag()) {
return (SHELL_ABORTED);
@@ -427,7 +491,7 @@ CascadeProcessVariables (
//
// Recurse to the next iteration. We know "our" variable's name.
//
- ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle, FoundVarName, FoundVarGuid, FoundOne);
+ ShellStatus = CascadeProcessVariables (Name, Guid, Type, FileHandle, FoundVarName, FoundVarGuid, FoundOne, StandardFormatOutput);
if (ShellGetExecutionBreakFlag() || (ShellStatus == SHELL_ABORTED)) {
SHELL_FREE_NON_NULL(FoundVarName);
@@ -459,50 +523,86 @@ CascadeProcessVariables (
Status = gRT->GetVariable (FoundVarName, &FoundVarGuid, &Atts, &DataSize, DataBuffer);
}
}
- if ((Type == DmpStoreDisplay) || (Type == DmpStoreSave)) {
//
// Last error check then print this variable out.
//
+ if (Type == DmpStoreDisplay) {
+ if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName != NULL)) {
+ AttrString = GetAttrType(Atts);
+ if (StandardFormatOutput) {
+ HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
+ if (HexString != NULL) {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO), gShellDebug1HiiHandle,
+ FoundVarName, &FoundVarGuid, Atts, DataSize,
+ BinaryToHexString (
+ DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
+ )
+ );
+ FreePool (HexString);
+ } else {
+ Status = EFI_OUT_OF_RESOURCES;
+ }
+ } else {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE), gShellDebug1HiiHandle,
+ AttrString, &FoundVarGuid, FoundVarName, DataSize
+ );
+ DumpHex (2, 0, DataSize, DataBuffer);
+ }
+ SHELL_FREE_NON_NULL (AttrString);
+ }
+ } else if (Type == DmpStoreSave) {
if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName != NULL)) {
- RetString = GetAttrType(Atts);
- ShellPrintHiiEx(
- -1,
- -1,
- NULL,
- STRING_TOKEN(STR_DMPSTORE_HEADER_LINE),
- gShellDebug1HiiHandle,
- RetString,
- &FoundVarGuid,
- FoundVarName,
- DataSize);
- if (Type == DmpStoreDisplay) {
- DumpHex(2, 0, DataSize, DataBuffer);
+ AttrString = GetAttrType (Atts);
+ if (StandardFormatOutput) {
+ HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
+ if (HexString != NULL) {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO), gShellDebug1HiiHandle,
+ FoundVarName, &FoundVarGuid, Atts, DataSize,
+ BinaryToHexString (
+ DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
+ )
+ );
+ FreePool (HexString);
+ } else {
+ Status = EFI_OUT_OF_RESOURCES;
+ }
} else {
- Status = AppendSingleVariableToFile (
- FileHandle,
- FoundVarName,
- &FoundVarGuid,
- Atts,
- (UINT32) DataSize,
- DataBuffer
- );
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE), gShellDebug1HiiHandle,
+ AttrString, &FoundVarGuid, FoundVarName, DataSize
+ );
}
- SHELL_FREE_NON_NULL(RetString);
+ Status = AppendSingleVariableToFile (
+ FileHandle,
+ FoundVarName,
+ &FoundVarGuid,
+ Atts,
+ (UINT32) DataSize,
+ DataBuffer
+ );
+ SHELL_FREE_NON_NULL (AttrString);
}
} else if (Type == DmpStoreDelete) {
//
// We only need name to delete it...
//
- ShellPrintHiiEx (
- -1,
- -1,
- NULL,
- STRING_TOKEN(STR_DMPSTORE_DELETE_LINE),
- gShellDebug1HiiHandle,
- &FoundVarGuid,
- FoundVarName,
- gRT->SetVariable (FoundVarName, &FoundVarGuid, Atts, 0, NULL)
- );
+ EFI_STATUS SetStatus = gRT->SetVariable (FoundVarName, &FoundVarGuid, Atts, 0, NULL);
+ if (StandardFormatOutput) {
+ if (SetStatus == EFI_SUCCESS) {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_NG_SFO), gShellDebug1HiiHandle,
+ FoundVarName, &FoundVarGuid
+ );
+ }
+ } else {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_DELETE_LINE), gShellDebug1HiiHandle,
+ &FoundVarGuid, FoundVarName, SetStatus
+ );
+ }
}
SHELL_FREE_NON_NULL(DataBuffer);
}
@@ -523,10 +623,11 @@ CascadeProcessVariables (
/**
Function to display or delete variables. This will set up and call into the recursive function.
- @param[in] Name The variable name of the EFI variable (or NULL).
- @param[in] Guid The GUID of the variable set (or NULL).
- @param[in] Type The operation type.
- @param[in] FileHandle The file to save or load variables.
+ @param[in] Name The variable name of the EFI variable (or NULL).
+ @param[in] Guid The GUID of the variable set (or NULL).
+ @param[in] Type The operation type.
+ @param[in] FileHandle The file to save or load variables.
+ @param[in] StandardFormatOutput TRUE indicates Standard-Format Output.
@retval SHELL_SUCCESS The operation was successful.
@retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
@@ -539,7 +640,8 @@ ProcessVariables (
IN CONST CHAR16 *Name OPTIONAL,
IN CONST EFI_GUID *Guid OPTIONAL,
IN DMP_STORE_TYPE Type,
- IN SHELL_FILE_HANDLE FileHandle OPTIONAL
+ IN SHELL_FILE_HANDLE FileHandle OPTIONAL,
+ IN BOOLEAN StandardFormatOutput
)
{
SHELL_STATUS ShellStatus;
@@ -550,10 +652,14 @@ ProcessVariables (
ShellStatus = SHELL_SUCCESS;
ZeroMem (&FoundVarGuid, sizeof(EFI_GUID));
+ if (StandardFormatOutput) {
+ ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN(STR_GEN_SFO_HEADER), gShellDebug1HiiHandle, L"dmpstore");
+ }
+
if (Type == DmpStoreLoad) {
- ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid, &Found);
+ ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid, &Found, StandardFormatOutput);
} else {
- ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle, NULL, FoundVarGuid, &Found);
+ ShellStatus = CascadeProcessVariables (Name, Guid, Type, FileHandle, NULL, FoundVarGuid, &Found, StandardFormatOutput);
}
if (!Found) {
@@ -561,13 +667,25 @@ ProcessVariables (
ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_OUT_MEM), gShellDebug1HiiHandle, L"dmpstore");
return (ShellStatus);
} else if (Name != NULL && Guid == NULL) {
- ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle, L"dmpstore", Name);
+ if (StandardFormatOutput) {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_N_SFO), gShellDebug1HiiHandle, Name);
+ } else {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle, L"dmpstore", Name);
+ }
} else if (Name != NULL && Guid != NULL) {
ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_GN), gShellDebug1HiiHandle, L"dmpstore", Guid, Name);
} else if (Name == NULL && Guid == NULL) {
- ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
+ if (StandardFormatOutput) {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_SFO), gShellDebug1HiiHandle);
+ } else {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
+ }
} else if (Name == NULL && Guid != NULL) {
- ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle, L"dmpstore", Guid);
+ if (StandardFormatOutput) {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_G_SFO), gShellDebug1HiiHandle, Guid);
+ } else {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle, L"dmpstore", Guid);
+ }
}
return (SHELL_NOT_FOUND);
}
@@ -580,6 +698,7 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
{L"-s", TypeValue},
{L"-all", TypeFlag},
{L"-guid", TypeValue},
+ {L"-sfo", TypeFlag},
{NULL, TypeMax}
};
@@ -608,12 +727,14 @@ ShellCommandRunDmpStore (
DMP_STORE_TYPE Type;
SHELL_FILE_HANDLE FileHandle;
EFI_FILE_INFO *FileInfo;
+ BOOLEAN StandardFormatOutput;
- ShellStatus = SHELL_SUCCESS;
- Package = NULL;
- FileHandle = NULL;
- File = NULL;
- Type = DmpStoreDisplay;
+ ShellStatus = SHELL_SUCCESS;
+ Package = NULL;
+ FileHandle = NULL;
+ File = NULL;
+ Type = DmpStoreDisplay;
+ StandardFormatOutput = FALSE;
Status = ShellCommandLineParse (ParamList, &Package, &ProblemParam, TRUE);
if (EFI_ERROR(Status)) {
@@ -742,6 +863,10 @@ ShellCommandRunDmpStore (
} else if (ShellCommandLineGetFlag(Package, L"-d")) {
Type = DmpStoreDelete;
}
+
+ if (ShellCommandLineGetFlag (Package,L"-sfo")) {
+ StandardFormatOutput = TRUE;
+ }
}
if (ShellStatus == SHELL_SUCCESS) {
@@ -750,7 +875,7 @@ ShellCommandRunDmpStore (
} else if (Type == DmpStoreLoad) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD), gShellDebug1HiiHandle, File);
}
- ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle);
+ ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle, StandardFormatOutput);
if ((Type == DmpStoreLoad) || (Type == DmpStoreSave)) {
ShellCloseFile (&FileHandle);
}
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
index 52c2af0..c97bd62 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
@@ -407,9 +407,14 @@
#string STR_DMPSTORE_HEADER_LINE #language en-US "Variable %H%s%N '%H%g%N:%H%s%N' DataSize = 0x%02x\r\n"
#string STR_DMPSTORE_DELETE_LINE #language en-US "Delete variable '%H%g%N:%H%s%N': %r\r\n"
#string STR_DMPSTORE_NO_VAR_FOUND #language en-US "%H%s%N: No matching variables found.\r\n"
+#string STR_DMPSTORE_NO_VAR_FOUND_SFO #language en-US "VariableInfo,\"\",\"\",\"\",\"\",\"\"\r\n"
#string STR_DMPSTORE_NO_VAR_FOUND_GN #language en-US "%H%s%N: No matching variables found. Guid %g, Name %s\r\n"
+#string STR_DMPSTORE_NO_VAR_FOUND_NG_SFO #language en-US "VariableInfo,\"%s\",\"%g\",\"\",\"\",\"\"\r\n"
#string STR_DMPSTORE_NO_VAR_FOUND_N #language en-US "%H%s%N: No matching variables found. Name %s\r\n"
+#string STR_DMPSTORE_NO_VAR_FOUND_N_SFO #language en-US #language en-US "VariableInfo,\"%s\",\"\",\"\",\"\",\"\"\r\n"
#string STR_DMPSTORE_NO_VAR_FOUND_G #language en-US "%H%s%N: No matching variables found. Guid %g\r\n"
+#string STR_DMPSTORE_NO_VAR_FOUND_G_SFO #language en-US "VariableInfo,\"\",\"%g\",\"\",\"\",\"\"\r\n"
+#string STR_DMPSTORE_VAR_SFO #language en-US "VariableInfo,\"%s\",\"%g\",\"0x%x\",\"0x%x\",\"%s\"\r\n"
#string STR_GET_HELP_COMP #language en-US ""
".TH comp 0 "Compare 2 files"\r\n"
--
2.9.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ShellPkg/dmpstore: Support "-sfo"
2016-11-11 17:21 ` Shah, Tapan
@ 2016-11-14 2:24 ` Ni, Ruiyu
0 siblings, 0 replies; 6+ messages in thread
From: Ni, Ruiyu @ 2016-11-14 2:24 UTC (permalink / raw)
To: Shah, Tapan, edk2-devel@lists.01.org; +Cc: Chen, Chen A, Carsey, Jaben
Comments below.
Thanks/Ray
> -----Original Message-----
> From: Shah, Tapan [mailto:tapandshah@hpe.com]
> Sent: Saturday, November 12, 2016 1:21 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Chen, Chen A <chen.a.chen@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>
> Subject: RE: [PATCH] ShellPkg/dmpstore: Support "-sfo"
>
> Two comments:
> 1. Missing help output update for -sfo support in dmpstore command.
Thanks for catching that. Will do that in V2 patch.
> 2. BinaryToHexString() is missing input parameter NULL and out of bound size
> check before access.
Will add assertion in V2 patch. Assertion is enough since it's internal function.
>
> -----Original Message-----
> From: Ruiyu Ni [mailto:ruiyu.ni@intel.com]
> Sent: Friday, November 11, 2016 4:14 AM
> To: edk2-devel@lists.01.org
> Cc: Chen A Chen <chen.a.chen@intel.com>; Jaben Carsey
> <jaben.carsey@intel.com>; Shah, Tapan <tapandshah@hpe.com>
> Subject: [PATCH] ShellPkg/dmpstore: Support "-sfo"
>
> The patch adds the "-sfo" support to "dmpstore" command.
>
> 1. For "-l" (load variable from file), when the variable content can be updated,
> the new variable data is displayed in SFO format, otherwise, the new variable
> data is not displayed. So that the SFO consumer can know which variables are
> updated by parsing the SFO.
>
> 2. For "-d" (delete variable), when the variable can be deleted successfully,
> only the variable name and GUID are displayed in SFO but the
> attribute/data/data size are displayed as empty to indicate the deletion
> happened; otherwise, the variable is not displayed.
>
> 3. For displaying variable, when the variable specified by name and GUID
> cannot be found, an error message is displayed; Otherwise, the SFO is
> displayed.
> E.g.: "dmpstore -guid GuidThatDoesntExist -sfo" produces output
> as:
> ShellCommand,"dmpstore"
> VariableInfo,"","GuidThatDoesntExist","","",""
>
> "dmpstore NameThatDoesntExist -guid GuidThatDoesntExist -sfo"
> produces output as:
> ShellCommand,"dmpstore"
> dmpstore: No matching variables found. Guid GuidThatDoesntExist, Name
> NameThatDoesntExist
>
> The difference between the above 2 cases is that former one only specifies
> the GUID, but the latter one specifies both name and GUID.
> Since not specifying GUID means to use GlobalVariableGuid, "dmpstore
> NameThatDoesntExist -sfo" produces the similar output as latter one.
> I personally prefer to always produce SFO output for both cases.
> But the above behavior is the discussion result between HPE engineers.
>
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Tapan Shah <tapandshah@hpe.com>
> ---
> .../Library/UefiShellDebug1CommandsLib/DmpStore.c | 269
> +++++++++++++++------
> .../UefiShellDebug1CommandsLib.uni | 5 +
> 2 files changed, 202 insertions(+), 72 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> index 3427c99..aa8ad09 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> @@ -2,7 +2,7 @@
> Main file for DmpStore shell Debug1 function.
>
> (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.<BR>
> - Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2005 - 2016, Intel Corporation. All rights
> + reserved.<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 @@ -82,12 +82,46 @@ GetAttrType ( }
>
> /**
> + Convert binary to hex format string.
> +
> + @param[in] BufferSize The size in bytes of the binary data.
> + @param[in] Buffer The binary data.
> + @param[in, out] HexString Hex format string.
> + @param[in] HexStringSize The size in bytes of the string.
> +
> + @return The hex format string.
> +**/
> +CHAR16*
> +BinaryToHexString (
> + IN VOID *Buffer,
> + IN UINTN BufferSize,
> + IN OUT CHAR16 *HexString,
> + IN UINTN HexStringSize
> + )
> +{
> + UINTN Index;
> + UINTN StringIndex;
> +
> + for (Index = 0, StringIndex = 0; Index < BufferSize; Index += 1) {
> + StringIndex +=
> + UnicodeSPrint (
> + &HexString[StringIndex],
> + HexStringSize - StringIndex * sizeof (CHAR16),
> + L"%02x",
> + ((UINT8 *) Buffer)[Index]
> + );
> + }
> + return HexString;
> +}
> +
> +/**
> Load the variable data from file and set to variable data base.
>
> - @param[in] FileHandle The file to be read.
> - @param[in] Name The name of the variables to be loaded.
> - @param[in] Guid The guid of the variables to be loaded.
> - @param[out] Found TRUE when at least one variable was loaded and
> set.
> + @param[in] FileHandle The file to be read.
> + @param[in] Name The name of the variables to be loaded.
> + @param[in] Guid The guid of the variables to be loaded.
> + @param[out] Found TRUE when at least one variable was loaded
> and set.
> + @param[in] StandardFormatOutput TRUE indicates Standard-Format
> Output.
>
> @retval SHELL_DEVICE_ERROR Cannot access the file.
> @retval SHELL_VOLUME_CORRUPTED The file is in bad format.
> @@ -99,7 +133,8 @@ LoadVariablesFromFile (
> IN SHELL_FILE_HANDLE FileHandle,
> IN CONST CHAR16 *Name,
> IN CONST EFI_GUID *Guid,
> - OUT BOOLEAN *Found
> + OUT BOOLEAN *Found,
> + IN BOOLEAN StandardFormatOutput
> )
> {
> EFI_STATUS Status;
> @@ -116,6 +151,7 @@ LoadVariablesFromFile (
> CHAR16 *Attributes;
> UINT8 *Buffer;
> UINT32 Crc32;
> + CHAR16 *HexString;
>
> Status = ShellGetFileSize (FileHandle, &FileSize);
> if (EFI_ERROR (Status)) {
> @@ -221,11 +257,6 @@ LoadVariablesFromFile (
> ((Guid == NULL) || CompareGuid (&Variable->Guid, Guid))
> ) {
> Attributes = GetAttrType (Variable->Attributes);
> - ShellPrintHiiEx (
> - -1, -1, NULL, STRING_TOKEN(STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> - Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
> - );
> - SHELL_FREE_NON_NULL(Attributes);
>
> *Found = TRUE;
> Status = gRT->SetVariable (
> @@ -236,8 +267,38 @@ LoadVariablesFromFile (
> Variable->Data
> );
> if (EFI_ERROR (Status)) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_LOAD_GEN_FAIL), gShellDebug1HiiHandle, L"dmpstore",
> Variable->Name, Status);
> + if (StandardFormatOutput) {
> + //
> + // Supress SFO to indicate the loading is failed.
> + //
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD_GEN_FAIL),
> gShellDebug1HiiHandle,
> + L"dmpstore", Variable->Name, Status
> + );
> + }
> + } else {
> + if (StandardFormatOutput) {
> + HexString = AllocatePool ((Variable->DataSize * 2 + 1) * sizeof
> (CHAR16));
> + if (HexString != NULL) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO),
> gShellDebug1HiiHandle,
> + Variable->Name, &Variable->Guid, Variable->Attributes, Variable-
> >DataSize,
> + BinaryToHexString (
> + Variable->Data, Variable->DataSize, HexString,
> + (Variable->DataSize * 2 + 1) * sizeof (CHAR16)
> + )
> + );
> + FreePool (HexString);
> + }
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> + Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
> + );
> + }
> }
> + SHELL_FREE_NON_NULL(Attributes);
> }
> }
>
> @@ -350,14 +411,15 @@ AppendSingleVariableToFile (
>
> This is necessary since once a delete happens GetNextVariableName() will
> work.
>
> - @param[in] Name The variable name of the EFI variable (or NULL).
> - @param[in] Guid The GUID of the variable set (or NULL).
> - @param[in] Type The operation type.
> - @param[in] FileHandle The file to operate on (or NULL).
> - @param[in] PrevName The previous variable name from
> GetNextVariableName. L"" to start.
> - @param[in] FoundVarGuid The previous GUID from
> GetNextVariableName. ignored at start.
> - @param[in] FoundOne If a VariableName or Guid was specified and one
> was printed or
> - deleted, then set this to TRUE, otherwise ignored.
> + @param[in] Name The variable name of the EFI variable (or NULL).
> + @param[in] Guid The GUID of the variable set (or NULL).
> + @param[in] Type The operation type.
> + @param[in] FileHandle The file to operate on (or NULL).
> + @param[in] PrevName The previous variable name from
> GetNextVariableName. L"" to start.
> + @param[in] FoundVarGuid The previous GUID from
> GetNextVariableName. ignored at start.
> + @param[in] FoundOne If a VariableName or Guid was specified and
> one was printed or
> + deleted, then set this to TRUE, otherwise ignored.
> + @param[in] StandardFormatOutput TRUE indicates Standard-Format
> Output.
>
> @retval SHELL_SUCCESS The operation was successful.
> @retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
> @@ -373,7 +435,8 @@ CascadeProcessVariables (
> IN EFI_FILE_PROTOCOL *FileHandle OPTIONAL,
> IN CONST CHAR16 * CONST PrevName,
> IN EFI_GUID FoundVarGuid,
> - IN BOOLEAN *FoundOne
> + IN BOOLEAN *FoundOne,
> + IN BOOLEAN StandardFormatOutput
> )
> {
> EFI_STATUS Status;
> @@ -383,7 +446,8 @@ CascadeProcessVariables (
> UINT32 Atts;
> SHELL_STATUS ShellStatus;
> UINTN NameSize;
> - CHAR16 *RetString;
> + CHAR16 *AttrString;
> + CHAR16 *HexString;
>
> if (ShellGetExecutionBreakFlag()) {
> return (SHELL_ABORTED);
> @@ -427,7 +491,7 @@ CascadeProcessVariables (
> //
> // Recurse to the next iteration. We know "our" variable's name.
> //
> - ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle,
> FoundVarName, FoundVarGuid, FoundOne);
> + ShellStatus = CascadeProcessVariables (Name, Guid, Type, FileHandle,
> + FoundVarName, FoundVarGuid, FoundOne, StandardFormatOutput);
>
> if (ShellGetExecutionBreakFlag() || (ShellStatus == SHELL_ABORTED)) {
> SHELL_FREE_NON_NULL(FoundVarName);
> @@ -459,50 +523,86 @@ CascadeProcessVariables (
> Status = gRT->GetVariable (FoundVarName, &FoundVarGuid, &Atts,
> &DataSize, DataBuffer);
> }
> }
> - if ((Type == DmpStoreDisplay) || (Type == DmpStoreSave)) {
> //
> // Last error check then print this variable out.
> //
> + if (Type == DmpStoreDisplay) {
> + if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName !=
> NULL)) {
> + AttrString = GetAttrType(Atts);
> + if (StandardFormatOutput) {
> + HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
> + if (HexString != NULL) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO),
> gShellDebug1HiiHandle,
> + FoundVarName, &FoundVarGuid, Atts, DataSize,
> + BinaryToHexString (
> + DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
> + )
> + );
> + FreePool (HexString);
> + } else {
> + Status = EFI_OUT_OF_RESOURCES;
> + }
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> + AttrString, &FoundVarGuid, FoundVarName, DataSize
> + );
> + DumpHex (2, 0, DataSize, DataBuffer);
> + }
> + SHELL_FREE_NON_NULL (AttrString);
> + }
> + } else if (Type == DmpStoreSave) {
> if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName !=
> NULL)) {
> - RetString = GetAttrType(Atts);
> - ShellPrintHiiEx(
> - -1,
> - -1,
> - NULL,
> - STRING_TOKEN(STR_DMPSTORE_HEADER_LINE),
> - gShellDebug1HiiHandle,
> - RetString,
> - &FoundVarGuid,
> - FoundVarName,
> - DataSize);
> - if (Type == DmpStoreDisplay) {
> - DumpHex(2, 0, DataSize, DataBuffer);
> + AttrString = GetAttrType (Atts);
> + if (StandardFormatOutput) {
> + HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
> + if (HexString != NULL) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO),
> gShellDebug1HiiHandle,
> + FoundVarName, &FoundVarGuid, Atts, DataSize,
> + BinaryToHexString (
> + DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
> + )
> + );
> + FreePool (HexString);
> + } else {
> + Status = EFI_OUT_OF_RESOURCES;
> + }
> } else {
> - Status = AppendSingleVariableToFile (
> - FileHandle,
> - FoundVarName,
> - &FoundVarGuid,
> - Atts,
> - (UINT32) DataSize,
> - DataBuffer
> - );
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> + AttrString, &FoundVarGuid, FoundVarName, DataSize
> + );
> }
> - SHELL_FREE_NON_NULL(RetString);
> + Status = AppendSingleVariableToFile (
> + FileHandle,
> + FoundVarName,
> + &FoundVarGuid,
> + Atts,
> + (UINT32) DataSize,
> + DataBuffer
> + );
> + SHELL_FREE_NON_NULL (AttrString);
> }
> } else if (Type == DmpStoreDelete) {
> //
> // We only need name to delete it...
> //
> - ShellPrintHiiEx (
> - -1,
> - -1,
> - NULL,
> - STRING_TOKEN(STR_DMPSTORE_DELETE_LINE),
> - gShellDebug1HiiHandle,
> - &FoundVarGuid,
> - FoundVarName,
> - gRT->SetVariable (FoundVarName, &FoundVarGuid, Atts, 0, NULL)
> - );
> + EFI_STATUS SetStatus = gRT->SetVariable (FoundVarName,
> &FoundVarGuid, Atts, 0, NULL);
> + if (StandardFormatOutput) {
> + if (SetStatus == EFI_SUCCESS) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_NG_SFO), gShellDebug1HiiHandle,
> + FoundVarName, &FoundVarGuid
> + );
> + }
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_DELETE_LINE),
> gShellDebug1HiiHandle,
> + &FoundVarGuid, FoundVarName, SetStatus
> + );
> + }
> }
> SHELL_FREE_NON_NULL(DataBuffer);
> }
> @@ -523,10 +623,11 @@ CascadeProcessVariables (
> /**
> Function to display or delete variables. This will set up and call into the
> recursive function.
>
> - @param[in] Name The variable name of the EFI variable (or NULL).
> - @param[in] Guid The GUID of the variable set (or NULL).
> - @param[in] Type The operation type.
> - @param[in] FileHandle The file to save or load variables.
> + @param[in] Name The variable name of the EFI variable (or NULL).
> + @param[in] Guid The GUID of the variable set (or NULL).
> + @param[in] Type The operation type.
> + @param[in] FileHandle The file to save or load variables.
> + @param[in] StandardFormatOutput TRUE indicates Standard-Format
> Output.
>
> @retval SHELL_SUCCESS The operation was successful.
> @retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
> @@ -539,7 +640,8 @@ ProcessVariables (
> IN CONST CHAR16 *Name OPTIONAL,
> IN CONST EFI_GUID *Guid OPTIONAL,
> IN DMP_STORE_TYPE Type,
> - IN SHELL_FILE_HANDLE FileHandle OPTIONAL
> + IN SHELL_FILE_HANDLE FileHandle OPTIONAL,
> + IN BOOLEAN StandardFormatOutput
> )
> {
> SHELL_STATUS ShellStatus;
> @@ -550,10 +652,14 @@ ProcessVariables (
> ShellStatus = SHELL_SUCCESS;
> ZeroMem (&FoundVarGuid, sizeof(EFI_GUID));
>
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN(STR_GEN_SFO_HEADER),
> + gShellDebug1HiiHandle, L"dmpstore"); }
> +
> if (Type == DmpStoreLoad) {
> - ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid, &Found);
> + ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid,
> + &Found, StandardFormatOutput);
> } else {
> - ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle,
> NULL, FoundVarGuid, &Found);
> + ShellStatus = CascadeProcessVariables (Name, Guid, Type,
> + FileHandle, NULL, FoundVarGuid, &Found, StandardFormatOutput);
> }
>
> if (!Found) {
> @@ -561,13 +667,25 @@ ProcessVariables (
> ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_OUT_MEM),
> gShellDebug1HiiHandle, L"dmpstore");
> return (ShellStatus);
> } else if (Name != NULL && Guid == NULL) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle,
> L"dmpstore", Name);
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_N_SFO), gShellDebug1HiiHandle,
> Name);
> + } else {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle,
> L"dmpstore", Name);
> + }
> } else if (Name != NULL && Guid != NULL) {
> ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_GN), gShellDebug1HiiHandle,
> L"dmpstore", Guid, Name);
> } else if (Name == NULL && Guid == NULL) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_SFO), gShellDebug1HiiHandle);
> + } else {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
> + }
> } else if (Name == NULL && Guid != NULL) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle,
> L"dmpstore", Guid);
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_G_SFO), gShellDebug1HiiHandle, Guid);
> + } else {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle,
> L"dmpstore", Guid);
> + }
> }
> return (SHELL_NOT_FOUND);
> }
> @@ -580,6 +698,7 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
> {L"-s", TypeValue},
> {L"-all", TypeFlag},
> {L"-guid", TypeValue},
> + {L"-sfo", TypeFlag},
> {NULL, TypeMax}
> };
>
> @@ -608,12 +727,14 @@ ShellCommandRunDmpStore (
> DMP_STORE_TYPE Type;
> SHELL_FILE_HANDLE FileHandle;
> EFI_FILE_INFO *FileInfo;
> + BOOLEAN StandardFormatOutput;
>
> - ShellStatus = SHELL_SUCCESS;
> - Package = NULL;
> - FileHandle = NULL;
> - File = NULL;
> - Type = DmpStoreDisplay;
> + ShellStatus = SHELL_SUCCESS;
> + Package = NULL;
> + FileHandle = NULL;
> + File = NULL;
> + Type = DmpStoreDisplay;
> + StandardFormatOutput = FALSE;
>
> Status = ShellCommandLineParse (ParamList, &Package, &ProblemParam,
> TRUE);
> if (EFI_ERROR(Status)) {
> @@ -742,6 +863,10 @@ ShellCommandRunDmpStore (
> } else if (ShellCommandLineGetFlag(Package, L"-d")) {
> Type = DmpStoreDelete;
> }
> +
> + if (ShellCommandLineGetFlag (Package,L"-sfo")) {
> + StandardFormatOutput = TRUE;
> + }
> }
>
> if (ShellStatus == SHELL_SUCCESS) { @@ -750,7 +875,7 @@
> ShellCommandRunDmpStore (
> } else if (Type == DmpStoreLoad) {
> ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD),
> gShellDebug1HiiHandle, File);
> }
> - ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle);
> + ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle,
> + StandardFormatOutput);
> if ((Type == DmpStoreLoad) || (Type == DmpStoreSave)) {
> ShellCloseFile (&FileHandle);
> }
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> index 52c2af0..c97bd62 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> d
> +++ sLib.uni
> @@ -407,9 +407,14 @@
> #string STR_DMPSTORE_HEADER_LINE #language en-US
> "Variable %H%s%N '%H%g%N:%H%s%N' DataSize = 0x%02x\r\n"
> #string STR_DMPSTORE_DELETE_LINE #language en-US "Delete variable
> '%H%g%N:%H%s%N': %r\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND #language en-US "%H%s%N:
> No matching variables found.\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_SFO #language en-US
> "VariableInfo,\"\",\"\",\"\",\"\",\"\"\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND_GN #language en-US
> "%H%s%N: No matching variables found. Guid %g, Name %s\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_NG_SFO #language en-US
> "VariableInfo,\"%s\",\"%g\",\"\",\"\",\"\"\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND_N #language en-US "%H%s%N:
> No matching variables found. Name %s\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_N_SFO #language en-US
> #language en-US "VariableInfo,\"%s\",\"\",\"\",\"\",\"\"\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND_G #language en-US "%H%s%N:
> No matching variables found. Guid %g\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_G_SFO #language en-US
> "VariableInfo,\"\",\"%g\",\"\",\"\",\"\"\r\n"
> +#string STR_DMPSTORE_VAR_SFO #language en-US
> "VariableInfo,\"%s\",\"%g\",\"0x%x\",\"0x%x\",\"%s\"\r\n"
>
> #string STR_GET_HELP_COMP #language en-US ""
> ".TH comp 0 "Compare 2 files"\r\n"
> --
> 2.9.0.windows.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ShellPkg/dmpstore: Support "-sfo"
2016-11-11 10:14 [PATCH] ShellPkg/dmpstore: Support "-sfo" Ruiyu Ni
2016-11-11 14:20 ` Carsey, Jaben
2016-11-11 17:21 ` Shah, Tapan
@ 2016-11-11 20:43 ` Shah, Tapan
2016-11-14 2:28 ` Ni, Ruiyu
2 siblings, 1 reply; 6+ messages in thread
From: Shah, Tapan @ 2016-11-11 20:43 UTC (permalink / raw)
To: Ruiyu Ni, edk2-devel@lists.01.org, Phillips, Chris J (Plano, TX)
Cc: Chen A Chen, Jaben Carsey
Ray,
Chris and I looked at the patch closely again and saw that your change also includes -sfo support for -l and -s flags which is not supported according to the latest spec. See below syntax from spec:
dmpstore [-b] [-d] [-all | (-guid guid)] [variable] [-sfo]
dmpstore [-all | (-guid guid)] [variable] [-s file]
dmpstore [-all | (-guid guid)] [variable] [-l file]
If we want to include -l and -s support with -sfo flag, then we need to define new SFO tables and it requires UEFI Shell spec. update. Currently non-sfo mode for -l and -s flags do not display variable data and current -sfo table has variable data column defined.
We also saw that -l/-s -sfo mode suppresses error and that's confusing from user prospective as suppressing error does not provide enough information to user about what happened for an individual variable. This should be handled with new SFO tables for -l/-s flags and do it later after Shell spec. update.
Thanks,
Tapan
-----Original Message-----
From: Shah, Tapan
Sent: Friday, November 11, 2016 11:21 AM
To: 'Ruiyu Ni' <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
Cc: Chen A Chen <chen.a.chen@intel.com>; Jaben Carsey <jaben.carsey@intel.com>
Subject: RE: [PATCH] ShellPkg/dmpstore: Support "-sfo"
Two comments:
1. Missing help output update for -sfo support in dmpstore command.
2. BinaryToHexString() is missing input parameter NULL and out of bound size check before access.
-----Original Message-----
From: Ruiyu Ni [mailto:ruiyu.ni@intel.com]
Sent: Friday, November 11, 2016 4:14 AM
To: edk2-devel@lists.01.org
Cc: Chen A Chen <chen.a.chen@intel.com>; Jaben Carsey <jaben.carsey@intel.com>; Shah, Tapan <tapandshah@hpe.com>
Subject: [PATCH] ShellPkg/dmpstore: Support "-sfo"
The patch adds the "-sfo" support to "dmpstore" command.
1. For "-l" (load variable from file), when the variable content can be updated, the new variable data is displayed in SFO format, otherwise, the new variable data is not displayed. So that the SFO consumer can know which variables are updated by parsing the SFO.
2. For "-d" (delete variable), when the variable can be deleted successfully, only the variable name and GUID are displayed in SFO but the attribute/data/data size are displayed as empty to indicate the deletion happened; otherwise, the variable is not displayed.
3. For displaying variable, when the variable specified by name and GUID cannot be found, an error message is displayed; Otherwise, the SFO is displayed.
E.g.: "dmpstore -guid GuidThatDoesntExist -sfo" produces output
as:
ShellCommand,"dmpstore"
VariableInfo,"","GuidThatDoesntExist","","",""
"dmpstore NameThatDoesntExist -guid GuidThatDoesntExist -sfo"
produces output as:
ShellCommand,"dmpstore"
dmpstore: No matching variables found. Guid GuidThatDoesntExist, Name NameThatDoesntExist
The difference between the above 2 cases is that former one only specifies the GUID, but the latter one specifies both name and GUID.
Since not specifying GUID means to use GlobalVariableGuid, "dmpstore NameThatDoesntExist -sfo" produces the similar output as latter one.
I personally prefer to always produce SFO output for both cases.
But the above behavior is the discussion result between HPE engineers.
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Tapan Shah <tapandshah@hpe.com>
---
.../Library/UefiShellDebug1CommandsLib/DmpStore.c | 269 +++++++++++++++------
.../UefiShellDebug1CommandsLib.uni | 5 +
2 files changed, 202 insertions(+), 72 deletions(-)
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
index 3427c99..aa8ad09 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
@@ -2,7 +2,7 @@
Main file for DmpStore shell Debug1 function.
(C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.<BR>
- Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2005 - 2016, Intel Corporation. All rights
+ reserved.<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 @@ -82,12 +82,46 @@ GetAttrType ( }
/**
+ Convert binary to hex format string.
+
+ @param[in] BufferSize The size in bytes of the binary data.
+ @param[in] Buffer The binary data.
+ @param[in, out] HexString Hex format string.
+ @param[in] HexStringSize The size in bytes of the string.
+
+ @return The hex format string.
+**/
+CHAR16*
+BinaryToHexString (
+ IN VOID *Buffer,
+ IN UINTN BufferSize,
+ IN OUT CHAR16 *HexString,
+ IN UINTN HexStringSize
+ )
+{
+ UINTN Index;
+ UINTN StringIndex;
+
+ for (Index = 0, StringIndex = 0; Index < BufferSize; Index += 1) {
+ StringIndex +=
+ UnicodeSPrint (
+ &HexString[StringIndex],
+ HexStringSize - StringIndex * sizeof (CHAR16),
+ L"%02x",
+ ((UINT8 *) Buffer)[Index]
+ );
+ }
+ return HexString;
+}
+
+/**
Load the variable data from file and set to variable data base.
- @param[in] FileHandle The file to be read.
- @param[in] Name The name of the variables to be loaded.
- @param[in] Guid The guid of the variables to be loaded.
- @param[out] Found TRUE when at least one variable was loaded and set.
+ @param[in] FileHandle The file to be read.
+ @param[in] Name The name of the variables to be loaded.
+ @param[in] Guid The guid of the variables to be loaded.
+ @param[out] Found TRUE when at least one variable was loaded and set.
+ @param[in] StandardFormatOutput TRUE indicates Standard-Format Output.
@retval SHELL_DEVICE_ERROR Cannot access the file.
@retval SHELL_VOLUME_CORRUPTED The file is in bad format.
@@ -99,7 +133,8 @@ LoadVariablesFromFile (
IN SHELL_FILE_HANDLE FileHandle,
IN CONST CHAR16 *Name,
IN CONST EFI_GUID *Guid,
- OUT BOOLEAN *Found
+ OUT BOOLEAN *Found,
+ IN BOOLEAN StandardFormatOutput
)
{
EFI_STATUS Status;
@@ -116,6 +151,7 @@ LoadVariablesFromFile (
CHAR16 *Attributes;
UINT8 *Buffer;
UINT32 Crc32;
+ CHAR16 *HexString;
Status = ShellGetFileSize (FileHandle, &FileSize);
if (EFI_ERROR (Status)) {
@@ -221,11 +257,6 @@ LoadVariablesFromFile (
((Guid == NULL) || CompareGuid (&Variable->Guid, Guid))
) {
Attributes = GetAttrType (Variable->Attributes);
- ShellPrintHiiEx (
- -1, -1, NULL, STRING_TOKEN(STR_DMPSTORE_HEADER_LINE), gShellDebug1HiiHandle,
- Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
- );
- SHELL_FREE_NON_NULL(Attributes);
*Found = TRUE;
Status = gRT->SetVariable (
@@ -236,8 +267,38 @@ LoadVariablesFromFile (
Variable->Data
);
if (EFI_ERROR (Status)) {
- ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD_GEN_FAIL), gShellDebug1HiiHandle, L"dmpstore", Variable->Name, Status);
+ if (StandardFormatOutput) {
+ //
+ // Supress SFO to indicate the loading is failed.
+ //
+ } else {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD_GEN_FAIL), gShellDebug1HiiHandle,
+ L"dmpstore", Variable->Name, Status
+ );
+ }
+ } else {
+ if (StandardFormatOutput) {
+ HexString = AllocatePool ((Variable->DataSize * 2 + 1) * sizeof (CHAR16));
+ if (HexString != NULL) {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO), gShellDebug1HiiHandle,
+ Variable->Name, &Variable->Guid, Variable->Attributes, Variable->DataSize,
+ BinaryToHexString (
+ Variable->Data, Variable->DataSize, HexString,
+ (Variable->DataSize * 2 + 1) * sizeof (CHAR16)
+ )
+ );
+ FreePool (HexString);
+ }
+ } else {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE), gShellDebug1HiiHandle,
+ Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
+ );
+ }
}
+ SHELL_FREE_NON_NULL(Attributes);
}
}
@@ -350,14 +411,15 @@ AppendSingleVariableToFile (
This is necessary since once a delete happens GetNextVariableName() will work.
- @param[in] Name The variable name of the EFI variable (or NULL).
- @param[in] Guid The GUID of the variable set (or NULL).
- @param[in] Type The operation type.
- @param[in] FileHandle The file to operate on (or NULL).
- @param[in] PrevName The previous variable name from GetNextVariableName. L"" to start.
- @param[in] FoundVarGuid The previous GUID from GetNextVariableName. ignored at start.
- @param[in] FoundOne If a VariableName or Guid was specified and one was printed or
- deleted, then set this to TRUE, otherwise ignored.
+ @param[in] Name The variable name of the EFI variable (or NULL).
+ @param[in] Guid The GUID of the variable set (or NULL).
+ @param[in] Type The operation type.
+ @param[in] FileHandle The file to operate on (or NULL).
+ @param[in] PrevName The previous variable name from GetNextVariableName. L"" to start.
+ @param[in] FoundVarGuid The previous GUID from GetNextVariableName. ignored at start.
+ @param[in] FoundOne If a VariableName or Guid was specified and one was printed or
+ deleted, then set this to TRUE, otherwise ignored.
+ @param[in] StandardFormatOutput TRUE indicates Standard-Format Output.
@retval SHELL_SUCCESS The operation was successful.
@retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
@@ -373,7 +435,8 @@ CascadeProcessVariables (
IN EFI_FILE_PROTOCOL *FileHandle OPTIONAL,
IN CONST CHAR16 * CONST PrevName,
IN EFI_GUID FoundVarGuid,
- IN BOOLEAN *FoundOne
+ IN BOOLEAN *FoundOne,
+ IN BOOLEAN StandardFormatOutput
)
{
EFI_STATUS Status;
@@ -383,7 +446,8 @@ CascadeProcessVariables (
UINT32 Atts;
SHELL_STATUS ShellStatus;
UINTN NameSize;
- CHAR16 *RetString;
+ CHAR16 *AttrString;
+ CHAR16 *HexString;
if (ShellGetExecutionBreakFlag()) {
return (SHELL_ABORTED);
@@ -427,7 +491,7 @@ CascadeProcessVariables (
//
// Recurse to the next iteration. We know "our" variable's name.
//
- ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle, FoundVarName, FoundVarGuid, FoundOne);
+ ShellStatus = CascadeProcessVariables (Name, Guid, Type, FileHandle,
+ FoundVarName, FoundVarGuid, FoundOne, StandardFormatOutput);
if (ShellGetExecutionBreakFlag() || (ShellStatus == SHELL_ABORTED)) {
SHELL_FREE_NON_NULL(FoundVarName);
@@ -459,50 +523,86 @@ CascadeProcessVariables (
Status = gRT->GetVariable (FoundVarName, &FoundVarGuid, &Atts, &DataSize, DataBuffer);
}
}
- if ((Type == DmpStoreDisplay) || (Type == DmpStoreSave)) {
//
// Last error check then print this variable out.
//
+ if (Type == DmpStoreDisplay) {
+ if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName != NULL)) {
+ AttrString = GetAttrType(Atts);
+ if (StandardFormatOutput) {
+ HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
+ if (HexString != NULL) {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO), gShellDebug1HiiHandle,
+ FoundVarName, &FoundVarGuid, Atts, DataSize,
+ BinaryToHexString (
+ DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
+ )
+ );
+ FreePool (HexString);
+ } else {
+ Status = EFI_OUT_OF_RESOURCES;
+ }
+ } else {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE), gShellDebug1HiiHandle,
+ AttrString, &FoundVarGuid, FoundVarName, DataSize
+ );
+ DumpHex (2, 0, DataSize, DataBuffer);
+ }
+ SHELL_FREE_NON_NULL (AttrString);
+ }
+ } else if (Type == DmpStoreSave) {
if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName != NULL)) {
- RetString = GetAttrType(Atts);
- ShellPrintHiiEx(
- -1,
- -1,
- NULL,
- STRING_TOKEN(STR_DMPSTORE_HEADER_LINE),
- gShellDebug1HiiHandle,
- RetString,
- &FoundVarGuid,
- FoundVarName,
- DataSize);
- if (Type == DmpStoreDisplay) {
- DumpHex(2, 0, DataSize, DataBuffer);
+ AttrString = GetAttrType (Atts);
+ if (StandardFormatOutput) {
+ HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
+ if (HexString != NULL) {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO), gShellDebug1HiiHandle,
+ FoundVarName, &FoundVarGuid, Atts, DataSize,
+ BinaryToHexString (
+ DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
+ )
+ );
+ FreePool (HexString);
+ } else {
+ Status = EFI_OUT_OF_RESOURCES;
+ }
} else {
- Status = AppendSingleVariableToFile (
- FileHandle,
- FoundVarName,
- &FoundVarGuid,
- Atts,
- (UINT32) DataSize,
- DataBuffer
- );
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE), gShellDebug1HiiHandle,
+ AttrString, &FoundVarGuid, FoundVarName, DataSize
+ );
}
- SHELL_FREE_NON_NULL(RetString);
+ Status = AppendSingleVariableToFile (
+ FileHandle,
+ FoundVarName,
+ &FoundVarGuid,
+ Atts,
+ (UINT32) DataSize,
+ DataBuffer
+ );
+ SHELL_FREE_NON_NULL (AttrString);
}
} else if (Type == DmpStoreDelete) {
//
// We only need name to delete it...
//
- ShellPrintHiiEx (
- -1,
- -1,
- NULL,
- STRING_TOKEN(STR_DMPSTORE_DELETE_LINE),
- gShellDebug1HiiHandle,
- &FoundVarGuid,
- FoundVarName,
- gRT->SetVariable (FoundVarName, &FoundVarGuid, Atts, 0, NULL)
- );
+ EFI_STATUS SetStatus = gRT->SetVariable (FoundVarName, &FoundVarGuid, Atts, 0, NULL);
+ if (StandardFormatOutput) {
+ if (SetStatus == EFI_SUCCESS) {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_NG_SFO), gShellDebug1HiiHandle,
+ FoundVarName, &FoundVarGuid
+ );
+ }
+ } else {
+ ShellPrintHiiEx (
+ -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_DELETE_LINE), gShellDebug1HiiHandle,
+ &FoundVarGuid, FoundVarName, SetStatus
+ );
+ }
}
SHELL_FREE_NON_NULL(DataBuffer);
}
@@ -523,10 +623,11 @@ CascadeProcessVariables (
/**
Function to display or delete variables. This will set up and call into the recursive function.
- @param[in] Name The variable name of the EFI variable (or NULL).
- @param[in] Guid The GUID of the variable set (or NULL).
- @param[in] Type The operation type.
- @param[in] FileHandle The file to save or load variables.
+ @param[in] Name The variable name of the EFI variable (or NULL).
+ @param[in] Guid The GUID of the variable set (or NULL).
+ @param[in] Type The operation type.
+ @param[in] FileHandle The file to save or load variables.
+ @param[in] StandardFormatOutput TRUE indicates Standard-Format Output.
@retval SHELL_SUCCESS The operation was successful.
@retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
@@ -539,7 +640,8 @@ ProcessVariables (
IN CONST CHAR16 *Name OPTIONAL,
IN CONST EFI_GUID *Guid OPTIONAL,
IN DMP_STORE_TYPE Type,
- IN SHELL_FILE_HANDLE FileHandle OPTIONAL
+ IN SHELL_FILE_HANDLE FileHandle OPTIONAL,
+ IN BOOLEAN StandardFormatOutput
)
{
SHELL_STATUS ShellStatus;
@@ -550,10 +652,14 @@ ProcessVariables (
ShellStatus = SHELL_SUCCESS;
ZeroMem (&FoundVarGuid, sizeof(EFI_GUID));
+ if (StandardFormatOutput) {
+ ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN(STR_GEN_SFO_HEADER),
+ gShellDebug1HiiHandle, L"dmpstore"); }
+
if (Type == DmpStoreLoad) {
- ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid, &Found);
+ ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid,
+ &Found, StandardFormatOutput);
} else {
- ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle, NULL, FoundVarGuid, &Found);
+ ShellStatus = CascadeProcessVariables (Name, Guid, Type,
+ FileHandle, NULL, FoundVarGuid, &Found, StandardFormatOutput);
}
if (!Found) {
@@ -561,13 +667,25 @@ ProcessVariables (
ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_OUT_MEM), gShellDebug1HiiHandle, L"dmpstore");
return (ShellStatus);
} else if (Name != NULL && Guid == NULL) {
- ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle, L"dmpstore", Name);
+ if (StandardFormatOutput) {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_N_SFO), gShellDebug1HiiHandle, Name);
+ } else {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle, L"dmpstore", Name);
+ }
} else if (Name != NULL && Guid != NULL) {
ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_GN), gShellDebug1HiiHandle, L"dmpstore", Guid, Name);
} else if (Name == NULL && Guid == NULL) {
- ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
+ if (StandardFormatOutput) {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_SFO), gShellDebug1HiiHandle);
+ } else {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
+ }
} else if (Name == NULL && Guid != NULL) {
- ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle, L"dmpstore", Guid);
+ if (StandardFormatOutput) {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_G_SFO), gShellDebug1HiiHandle, Guid);
+ } else {
+ ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle, L"dmpstore", Guid);
+ }
}
return (SHELL_NOT_FOUND);
}
@@ -580,6 +698,7 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
{L"-s", TypeValue},
{L"-all", TypeFlag},
{L"-guid", TypeValue},
+ {L"-sfo", TypeFlag},
{NULL, TypeMax}
};
@@ -608,12 +727,14 @@ ShellCommandRunDmpStore (
DMP_STORE_TYPE Type;
SHELL_FILE_HANDLE FileHandle;
EFI_FILE_INFO *FileInfo;
+ BOOLEAN StandardFormatOutput;
- ShellStatus = SHELL_SUCCESS;
- Package = NULL;
- FileHandle = NULL;
- File = NULL;
- Type = DmpStoreDisplay;
+ ShellStatus = SHELL_SUCCESS;
+ Package = NULL;
+ FileHandle = NULL;
+ File = NULL;
+ Type = DmpStoreDisplay;
+ StandardFormatOutput = FALSE;
Status = ShellCommandLineParse (ParamList, &Package, &ProblemParam, TRUE);
if (EFI_ERROR(Status)) {
@@ -742,6 +863,10 @@ ShellCommandRunDmpStore (
} else if (ShellCommandLineGetFlag(Package, L"-d")) {
Type = DmpStoreDelete;
}
+
+ if (ShellCommandLineGetFlag (Package,L"-sfo")) {
+ StandardFormatOutput = TRUE;
+ }
}
if (ShellStatus == SHELL_SUCCESS) { @@ -750,7 +875,7 @@ ShellCommandRunDmpStore (
} else if (Type == DmpStoreLoad) {
ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD), gShellDebug1HiiHandle, File);
}
- ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle);
+ ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle,
+ StandardFormatOutput);
if ((Type == DmpStoreLoad) || (Type == DmpStoreSave)) {
ShellCloseFile (&FileHandle);
}
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
index 52c2af0..c97bd62 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Command
+++ sLib.uni
@@ -407,9 +407,14 @@
#string STR_DMPSTORE_HEADER_LINE #language en-US "Variable %H%s%N '%H%g%N:%H%s%N' DataSize = 0x%02x\r\n"
#string STR_DMPSTORE_DELETE_LINE #language en-US "Delete variable '%H%g%N:%H%s%N': %r\r\n"
#string STR_DMPSTORE_NO_VAR_FOUND #language en-US "%H%s%N: No matching variables found.\r\n"
+#string STR_DMPSTORE_NO_VAR_FOUND_SFO #language en-US "VariableInfo,\"\",\"\",\"\",\"\",\"\"\r\n"
#string STR_DMPSTORE_NO_VAR_FOUND_GN #language en-US "%H%s%N: No matching variables found. Guid %g, Name %s\r\n"
+#string STR_DMPSTORE_NO_VAR_FOUND_NG_SFO #language en-US "VariableInfo,\"%s\",\"%g\",\"\",\"\",\"\"\r\n"
#string STR_DMPSTORE_NO_VAR_FOUND_N #language en-US "%H%s%N: No matching variables found. Name %s\r\n"
+#string STR_DMPSTORE_NO_VAR_FOUND_N_SFO #language en-US #language en-US "VariableInfo,\"%s\",\"\",\"\",\"\",\"\"\r\n"
#string STR_DMPSTORE_NO_VAR_FOUND_G #language en-US "%H%s%N: No matching variables found. Guid %g\r\n"
+#string STR_DMPSTORE_NO_VAR_FOUND_G_SFO #language en-US "VariableInfo,\"\",\"%g\",\"\",\"\",\"\"\r\n"
+#string STR_DMPSTORE_VAR_SFO #language en-US "VariableInfo,\"%s\",\"%g\",\"0x%x\",\"0x%x\",\"%s\"\r\n"
#string STR_GET_HELP_COMP #language en-US ""
".TH comp 0 "Compare 2 files"\r\n"
--
2.9.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ShellPkg/dmpstore: Support "-sfo"
2016-11-11 20:43 ` Shah, Tapan
@ 2016-11-14 2:28 ` Ni, Ruiyu
0 siblings, 0 replies; 6+ messages in thread
From: Ni, Ruiyu @ 2016-11-14 2:28 UTC (permalink / raw)
To: Shah, Tapan, edk2-devel@lists.01.org,
Phillips, Chris J (Plano, TX)
Cc: Carsey, Jaben, Chen, Chen A
Yes I just saw that difference in Spec.
I agree to remove the sfo support for -s and -l.
But I do not think we need to define new SFO table.
I think the V1 patch's implementation can satisfy caller's information needs.
For -s, displaying the variable content in SFO is to tell caller that the exactly same
content is saved to file.
For -l, only displaying the variable content in SFO is to tell caller that the variable
listed in SFO is updated to the content shown in SFO.
Thanks/Ray
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Shah, Tapan
> Sent: Saturday, November 12, 2016 4:43 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org; Phillips, Chris J
> (Plano, TX) <chrisp@hpe.com>
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Chen, Chen A
> <chen.a.chen@intel.com>
> Subject: Re: [edk2] [PATCH] ShellPkg/dmpstore: Support "-sfo"
>
> Ray,
> Chris and I looked at the patch closely again and saw that your change also
> includes -sfo support for -l and -s flags which is not supported according to
> the latest spec. See below syntax from spec:
>
> dmpstore [-b] [-d] [-all | (-guid guid)] [variable] [-sfo] dmpstore [-all | (-guid
> guid)] [variable] [-s file] dmpstore [-all | (-guid guid)] [variable] [-l file]
>
> If we want to include -l and -s support with -sfo flag, then we need to define
> new SFO tables and it requires UEFI Shell spec. update. Currently non-sfo
> mode for -l and -s flags do not display variable data and current -sfo table has
> variable data column defined.
>
> We also saw that -l/-s -sfo mode suppresses error and that's confusing from
> user prospective as suppressing error does not provide enough information
> to user about what happened for an individual variable. This should be
> handled with new SFO tables for -l/-s flags and do it later after Shell spec.
> update.
>
> Thanks,
> Tapan
>
> -----Original Message-----
> From: Shah, Tapan
> Sent: Friday, November 11, 2016 11:21 AM
> To: 'Ruiyu Ni' <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Chen A Chen <chen.a.chen@intel.com>; Jaben Carsey
> <jaben.carsey@intel.com>
> Subject: RE: [PATCH] ShellPkg/dmpstore: Support "-sfo"
>
> Two comments:
> 1. Missing help output update for -sfo support in dmpstore command.
> 2. BinaryToHexString() is missing input parameter NULL and out of bound size
> check before access.
>
> -----Original Message-----
> From: Ruiyu Ni [mailto:ruiyu.ni@intel.com]
> Sent: Friday, November 11, 2016 4:14 AM
> To: edk2-devel@lists.01.org
> Cc: Chen A Chen <chen.a.chen@intel.com>; Jaben Carsey
> <jaben.carsey@intel.com>; Shah, Tapan <tapandshah@hpe.com>
> Subject: [PATCH] ShellPkg/dmpstore: Support "-sfo"
>
> The patch adds the "-sfo" support to "dmpstore" command.
>
> 1. For "-l" (load variable from file), when the variable content can be updated,
> the new variable data is displayed in SFO format, otherwise, the new variable
> data is not displayed. So that the SFO consumer can know which variables are
> updated by parsing the SFO.
>
> 2. For "-d" (delete variable), when the variable can be deleted successfully,
> only the variable name and GUID are displayed in SFO but the
> attribute/data/data size are displayed as empty to indicate the deletion
> happened; otherwise, the variable is not displayed.
>
> 3. For displaying variable, when the variable specified by name and GUID
> cannot be found, an error message is displayed; Otherwise, the SFO is
> displayed.
> E.g.: "dmpstore -guid GuidThatDoesntExist -sfo" produces output
> as:
> ShellCommand,"dmpstore"
> VariableInfo,"","GuidThatDoesntExist","","",""
>
> "dmpstore NameThatDoesntExist -guid GuidThatDoesntExist -sfo"
> produces output as:
> ShellCommand,"dmpstore"
> dmpstore: No matching variables found. Guid GuidThatDoesntExist, Name
> NameThatDoesntExist
>
> The difference between the above 2 cases is that former one only specifies
> the GUID, but the latter one specifies both name and GUID.
> Since not specifying GUID means to use GlobalVariableGuid, "dmpstore
> NameThatDoesntExist -sfo" produces the similar output as latter one.
> I personally prefer to always produce SFO output for both cases.
> But the above behavior is the discussion result between HPE engineers.
>
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Tapan Shah <tapandshah@hpe.com>
> ---
> .../Library/UefiShellDebug1CommandsLib/DmpStore.c | 269
> +++++++++++++++------
> .../UefiShellDebug1CommandsLib.uni | 5 +
> 2 files changed, 202 insertions(+), 72 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> index 3427c99..aa8ad09 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> @@ -2,7 +2,7 @@
> Main file for DmpStore shell Debug1 function.
>
> (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.<BR>
> - Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2005 - 2016, Intel Corporation. All rights
> + reserved.<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 @@ -82,12 +82,46 @@ GetAttrType ( }
>
> /**
> + Convert binary to hex format string.
> +
> + @param[in] BufferSize The size in bytes of the binary data.
> + @param[in] Buffer The binary data.
> + @param[in, out] HexString Hex format string.
> + @param[in] HexStringSize The size in bytes of the string.
> +
> + @return The hex format string.
> +**/
> +CHAR16*
> +BinaryToHexString (
> + IN VOID *Buffer,
> + IN UINTN BufferSize,
> + IN OUT CHAR16 *HexString,
> + IN UINTN HexStringSize
> + )
> +{
> + UINTN Index;
> + UINTN StringIndex;
> +
> + for (Index = 0, StringIndex = 0; Index < BufferSize; Index += 1) {
> + StringIndex +=
> + UnicodeSPrint (
> + &HexString[StringIndex],
> + HexStringSize - StringIndex * sizeof (CHAR16),
> + L"%02x",
> + ((UINT8 *) Buffer)[Index]
> + );
> + }
> + return HexString;
> +}
> +
> +/**
> Load the variable data from file and set to variable data base.
>
> - @param[in] FileHandle The file to be read.
> - @param[in] Name The name of the variables to be loaded.
> - @param[in] Guid The guid of the variables to be loaded.
> - @param[out] Found TRUE when at least one variable was loaded and
> set.
> + @param[in] FileHandle The file to be read.
> + @param[in] Name The name of the variables to be loaded.
> + @param[in] Guid The guid of the variables to be loaded.
> + @param[out] Found TRUE when at least one variable was loaded
> and set.
> + @param[in] StandardFormatOutput TRUE indicates Standard-Format
> Output.
>
> @retval SHELL_DEVICE_ERROR Cannot access the file.
> @retval SHELL_VOLUME_CORRUPTED The file is in bad format.
> @@ -99,7 +133,8 @@ LoadVariablesFromFile (
> IN SHELL_FILE_HANDLE FileHandle,
> IN CONST CHAR16 *Name,
> IN CONST EFI_GUID *Guid,
> - OUT BOOLEAN *Found
> + OUT BOOLEAN *Found,
> + IN BOOLEAN StandardFormatOutput
> )
> {
> EFI_STATUS Status;
> @@ -116,6 +151,7 @@ LoadVariablesFromFile (
> CHAR16 *Attributes;
> UINT8 *Buffer;
> UINT32 Crc32;
> + CHAR16 *HexString;
>
> Status = ShellGetFileSize (FileHandle, &FileSize);
> if (EFI_ERROR (Status)) {
> @@ -221,11 +257,6 @@ LoadVariablesFromFile (
> ((Guid == NULL) || CompareGuid (&Variable->Guid, Guid))
> ) {
> Attributes = GetAttrType (Variable->Attributes);
> - ShellPrintHiiEx (
> - -1, -1, NULL, STRING_TOKEN(STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> - Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
> - );
> - SHELL_FREE_NON_NULL(Attributes);
>
> *Found = TRUE;
> Status = gRT->SetVariable (
> @@ -236,8 +267,38 @@ LoadVariablesFromFile (
> Variable->Data
> );
> if (EFI_ERROR (Status)) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_LOAD_GEN_FAIL), gShellDebug1HiiHandle, L"dmpstore",
> Variable->Name, Status);
> + if (StandardFormatOutput) {
> + //
> + // Supress SFO to indicate the loading is failed.
> + //
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD_GEN_FAIL),
> gShellDebug1HiiHandle,
> + L"dmpstore", Variable->Name, Status
> + );
> + }
> + } else {
> + if (StandardFormatOutput) {
> + HexString = AllocatePool ((Variable->DataSize * 2 + 1) * sizeof
> (CHAR16));
> + if (HexString != NULL) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO),
> gShellDebug1HiiHandle,
> + Variable->Name, &Variable->Guid, Variable->Attributes, Variable-
> >DataSize,
> + BinaryToHexString (
> + Variable->Data, Variable->DataSize, HexString,
> + (Variable->DataSize * 2 + 1) * sizeof (CHAR16)
> + )
> + );
> + FreePool (HexString);
> + }
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> + Attributes, &Variable->Guid, Variable->Name, Variable->DataSize
> + );
> + }
> }
> + SHELL_FREE_NON_NULL(Attributes);
> }
> }
>
> @@ -350,14 +411,15 @@ AppendSingleVariableToFile (
>
> This is necessary since once a delete happens GetNextVariableName() will
> work.
>
> - @param[in] Name The variable name of the EFI variable (or NULL).
> - @param[in] Guid The GUID of the variable set (or NULL).
> - @param[in] Type The operation type.
> - @param[in] FileHandle The file to operate on (or NULL).
> - @param[in] PrevName The previous variable name from
> GetNextVariableName. L"" to start.
> - @param[in] FoundVarGuid The previous GUID from
> GetNextVariableName. ignored at start.
> - @param[in] FoundOne If a VariableName or Guid was specified and one
> was printed or
> - deleted, then set this to TRUE, otherwise ignored.
> + @param[in] Name The variable name of the EFI variable (or NULL).
> + @param[in] Guid The GUID of the variable set (or NULL).
> + @param[in] Type The operation type.
> + @param[in] FileHandle The file to operate on (or NULL).
> + @param[in] PrevName The previous variable name from
> GetNextVariableName. L"" to start.
> + @param[in] FoundVarGuid The previous GUID from
> GetNextVariableName. ignored at start.
> + @param[in] FoundOne If a VariableName or Guid was specified and
> one was printed or
> + deleted, then set this to TRUE, otherwise ignored.
> + @param[in] StandardFormatOutput TRUE indicates Standard-Format
> Output.
>
> @retval SHELL_SUCCESS The operation was successful.
> @retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
> @@ -373,7 +435,8 @@ CascadeProcessVariables (
> IN EFI_FILE_PROTOCOL *FileHandle OPTIONAL,
> IN CONST CHAR16 * CONST PrevName,
> IN EFI_GUID FoundVarGuid,
> - IN BOOLEAN *FoundOne
> + IN BOOLEAN *FoundOne,
> + IN BOOLEAN StandardFormatOutput
> )
> {
> EFI_STATUS Status;
> @@ -383,7 +446,8 @@ CascadeProcessVariables (
> UINT32 Atts;
> SHELL_STATUS ShellStatus;
> UINTN NameSize;
> - CHAR16 *RetString;
> + CHAR16 *AttrString;
> + CHAR16 *HexString;
>
> if (ShellGetExecutionBreakFlag()) {
> return (SHELL_ABORTED);
> @@ -427,7 +491,7 @@ CascadeProcessVariables (
> //
> // Recurse to the next iteration. We know "our" variable's name.
> //
> - ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle,
> FoundVarName, FoundVarGuid, FoundOne);
> + ShellStatus = CascadeProcessVariables (Name, Guid, Type, FileHandle,
> + FoundVarName, FoundVarGuid, FoundOne, StandardFormatOutput);
>
> if (ShellGetExecutionBreakFlag() || (ShellStatus == SHELL_ABORTED)) {
> SHELL_FREE_NON_NULL(FoundVarName);
> @@ -459,50 +523,86 @@ CascadeProcessVariables (
> Status = gRT->GetVariable (FoundVarName, &FoundVarGuid, &Atts,
> &DataSize, DataBuffer);
> }
> }
> - if ((Type == DmpStoreDisplay) || (Type == DmpStoreSave)) {
> //
> // Last error check then print this variable out.
> //
> + if (Type == DmpStoreDisplay) {
> + if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName !=
> NULL)) {
> + AttrString = GetAttrType(Atts);
> + if (StandardFormatOutput) {
> + HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
> + if (HexString != NULL) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO),
> gShellDebug1HiiHandle,
> + FoundVarName, &FoundVarGuid, Atts, DataSize,
> + BinaryToHexString (
> + DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
> + )
> + );
> + FreePool (HexString);
> + } else {
> + Status = EFI_OUT_OF_RESOURCES;
> + }
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> + AttrString, &FoundVarGuid, FoundVarName, DataSize
> + );
> + DumpHex (2, 0, DataSize, DataBuffer);
> + }
> + SHELL_FREE_NON_NULL (AttrString);
> + }
> + } else if (Type == DmpStoreSave) {
> if (!EFI_ERROR(Status) && (DataBuffer != NULL) && (FoundVarName !=
> NULL)) {
> - RetString = GetAttrType(Atts);
> - ShellPrintHiiEx(
> - -1,
> - -1,
> - NULL,
> - STRING_TOKEN(STR_DMPSTORE_HEADER_LINE),
> - gShellDebug1HiiHandle,
> - RetString,
> - &FoundVarGuid,
> - FoundVarName,
> - DataSize);
> - if (Type == DmpStoreDisplay) {
> - DumpHex(2, 0, DataSize, DataBuffer);
> + AttrString = GetAttrType (Atts);
> + if (StandardFormatOutput) {
> + HexString = AllocatePool ((DataSize * 2 + 1) * sizeof (CHAR16));
> + if (HexString != NULL) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_VAR_SFO),
> gShellDebug1HiiHandle,
> + FoundVarName, &FoundVarGuid, Atts, DataSize,
> + BinaryToHexString (
> + DataBuffer, DataSize, HexString, (DataSize * 2 + 1) * sizeof (CHAR16)
> + )
> + );
> + FreePool (HexString);
> + } else {
> + Status = EFI_OUT_OF_RESOURCES;
> + }
> } else {
> - Status = AppendSingleVariableToFile (
> - FileHandle,
> - FoundVarName,
> - &FoundVarGuid,
> - Atts,
> - (UINT32) DataSize,
> - DataBuffer
> - );
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_HEADER_LINE),
> gShellDebug1HiiHandle,
> + AttrString, &FoundVarGuid, FoundVarName, DataSize
> + );
> }
> - SHELL_FREE_NON_NULL(RetString);
> + Status = AppendSingleVariableToFile (
> + FileHandle,
> + FoundVarName,
> + &FoundVarGuid,
> + Atts,
> + (UINT32) DataSize,
> + DataBuffer
> + );
> + SHELL_FREE_NON_NULL (AttrString);
> }
> } else if (Type == DmpStoreDelete) {
> //
> // We only need name to delete it...
> //
> - ShellPrintHiiEx (
> - -1,
> - -1,
> - NULL,
> - STRING_TOKEN(STR_DMPSTORE_DELETE_LINE),
> - gShellDebug1HiiHandle,
> - &FoundVarGuid,
> - FoundVarName,
> - gRT->SetVariable (FoundVarName, &FoundVarGuid, Atts, 0, NULL)
> - );
> + EFI_STATUS SetStatus = gRT->SetVariable (FoundVarName,
> &FoundVarGuid, Atts, 0, NULL);
> + if (StandardFormatOutput) {
> + if (SetStatus == EFI_SUCCESS) {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_NG_SFO), gShellDebug1HiiHandle,
> + FoundVarName, &FoundVarGuid
> + );
> + }
> + } else {
> + ShellPrintHiiEx (
> + -1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_DELETE_LINE),
> gShellDebug1HiiHandle,
> + &FoundVarGuid, FoundVarName, SetStatus
> + );
> + }
> }
> SHELL_FREE_NON_NULL(DataBuffer);
> }
> @@ -523,10 +623,11 @@ CascadeProcessVariables (
> /**
> Function to display or delete variables. This will set up and call into the
> recursive function.
>
> - @param[in] Name The variable name of the EFI variable (or NULL).
> - @param[in] Guid The GUID of the variable set (or NULL).
> - @param[in] Type The operation type.
> - @param[in] FileHandle The file to save or load variables.
> + @param[in] Name The variable name of the EFI variable (or NULL).
> + @param[in] Guid The GUID of the variable set (or NULL).
> + @param[in] Type The operation type.
> + @param[in] FileHandle The file to save or load variables.
> + @param[in] StandardFormatOutput TRUE indicates Standard-Format
> Output.
>
> @retval SHELL_SUCCESS The operation was successful.
> @retval SHELL_OUT_OF_RESOURCES A memorty allocation failed.
> @@ -539,7 +640,8 @@ ProcessVariables (
> IN CONST CHAR16 *Name OPTIONAL,
> IN CONST EFI_GUID *Guid OPTIONAL,
> IN DMP_STORE_TYPE Type,
> - IN SHELL_FILE_HANDLE FileHandle OPTIONAL
> + IN SHELL_FILE_HANDLE FileHandle OPTIONAL,
> + IN BOOLEAN StandardFormatOutput
> )
> {
> SHELL_STATUS ShellStatus;
> @@ -550,10 +652,14 @@ ProcessVariables (
> ShellStatus = SHELL_SUCCESS;
> ZeroMem (&FoundVarGuid, sizeof(EFI_GUID));
>
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN(STR_GEN_SFO_HEADER),
> + gShellDebug1HiiHandle, L"dmpstore"); }
> +
> if (Type == DmpStoreLoad) {
> - ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid, &Found);
> + ShellStatus = LoadVariablesFromFile (FileHandle, Name, Guid,
> + &Found, StandardFormatOutput);
> } else {
> - ShellStatus = CascadeProcessVariables(Name, Guid, Type, FileHandle,
> NULL, FoundVarGuid, &Found);
> + ShellStatus = CascadeProcessVariables (Name, Guid, Type,
> + FileHandle, NULL, FoundVarGuid, &Found, StandardFormatOutput);
> }
>
> if (!Found) {
> @@ -561,13 +667,25 @@ ProcessVariables (
> ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_OUT_MEM),
> gShellDebug1HiiHandle, L"dmpstore");
> return (ShellStatus);
> } else if (Name != NULL && Guid == NULL) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle,
> L"dmpstore", Name);
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_N_SFO), gShellDebug1HiiHandle,
> Name);
> + } else {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_N), gShellDebug1HiiHandle,
> L"dmpstore", Name);
> + }
> } else if (Name != NULL && Guid != NULL) {
> ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_GN), gShellDebug1HiiHandle,
> L"dmpstore", Guid, Name);
> } else if (Name == NULL && Guid == NULL) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_SFO), gShellDebug1HiiHandle);
> + } else {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND), gShellDebug1HiiHandle, L"dmpstore");
> + }
> } else if (Name == NULL && Guid != NULL) {
> - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle,
> L"dmpstore", Guid);
> + if (StandardFormatOutput) {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_G_SFO), gShellDebug1HiiHandle, Guid);
> + } else {
> + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DMPSTORE_NO_VAR_FOUND_G), gShellDebug1HiiHandle,
> L"dmpstore", Guid);
> + }
> }
> return (SHELL_NOT_FOUND);
> }
> @@ -580,6 +698,7 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
> {L"-s", TypeValue},
> {L"-all", TypeFlag},
> {L"-guid", TypeValue},
> + {L"-sfo", TypeFlag},
> {NULL, TypeMax}
> };
>
> @@ -608,12 +727,14 @@ ShellCommandRunDmpStore (
> DMP_STORE_TYPE Type;
> SHELL_FILE_HANDLE FileHandle;
> EFI_FILE_INFO *FileInfo;
> + BOOLEAN StandardFormatOutput;
>
> - ShellStatus = SHELL_SUCCESS;
> - Package = NULL;
> - FileHandle = NULL;
> - File = NULL;
> - Type = DmpStoreDisplay;
> + ShellStatus = SHELL_SUCCESS;
> + Package = NULL;
> + FileHandle = NULL;
> + File = NULL;
> + Type = DmpStoreDisplay;
> + StandardFormatOutput = FALSE;
>
> Status = ShellCommandLineParse (ParamList, &Package, &ProblemParam,
> TRUE);
> if (EFI_ERROR(Status)) {
> @@ -742,6 +863,10 @@ ShellCommandRunDmpStore (
> } else if (ShellCommandLineGetFlag(Package, L"-d")) {
> Type = DmpStoreDelete;
> }
> +
> + if (ShellCommandLineGetFlag (Package,L"-sfo")) {
> + StandardFormatOutput = TRUE;
> + }
> }
>
> if (ShellStatus == SHELL_SUCCESS) { @@ -750,7 +875,7 @@
> ShellCommandRunDmpStore (
> } else if (Type == DmpStoreLoad) {
> ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DMPSTORE_LOAD),
> gShellDebug1HiiHandle, File);
> }
> - ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle);
> + ShellStatus = ProcessVariables (Name, Guid, Type, FileHandle,
> + StandardFormatOutput);
> if ((Type == DmpStoreLoad) || (Type == DmpStoreSave)) {
> ShellCloseFile (&FileHandle);
> }
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> index 52c2af0..c97bd62 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> d
> +++ sLib.uni
> @@ -407,9 +407,14 @@
> #string STR_DMPSTORE_HEADER_LINE #language en-US
> "Variable %H%s%N '%H%g%N:%H%s%N' DataSize = 0x%02x\r\n"
> #string STR_DMPSTORE_DELETE_LINE #language en-US "Delete variable
> '%H%g%N:%H%s%N': %r\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND #language en-US "%H%s%N:
> No matching variables found.\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_SFO #language en-US
> "VariableInfo,\"\",\"\",\"\",\"\",\"\"\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND_GN #language en-US
> "%H%s%N: No matching variables found. Guid %g, Name %s\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_NG_SFO #language en-US
> "VariableInfo,\"%s\",\"%g\",\"\",\"\",\"\"\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND_N #language en-US "%H%s%N:
> No matching variables found. Name %s\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_N_SFO #language en-US
> #language en-US "VariableInfo,\"%s\",\"\",\"\",\"\",\"\"\r\n"
> #string STR_DMPSTORE_NO_VAR_FOUND_G #language en-US "%H%s%N:
> No matching variables found. Guid %g\r\n"
> +#string STR_DMPSTORE_NO_VAR_FOUND_G_SFO #language en-US
> "VariableInfo,\"\",\"%g\",\"\",\"\",\"\"\r\n"
> +#string STR_DMPSTORE_VAR_SFO #language en-US
> "VariableInfo,\"%s\",\"%g\",\"0x%x\",\"0x%x\",\"%s\"\r\n"
>
> #string STR_GET_HELP_COMP #language en-US ""
> ".TH comp 0 "Compare 2 files"\r\n"
> --
> 2.9.0.windows.1
>
> _______________________________________________
> 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