Re: [edk2-devel] [PATCH v3 5/7] ShellPkg/AcpiView: Refactor DumpAcpiTableToFile

2020-06-22 Thread Gao, Zhichao
Reviewed-by: Zhichao Gao 

Thanks,
Zhichao

> -Original Message-
> From: Tomas Pilar 
> Sent: Monday, June 15, 2020 10:04 PM
> To: devel@edk2.groups.io
> Cc: n...@arm.com; Ni, Ray ; Gao, Zhichao
> 
> Subject: [PATCH v3 5/7] ShellPkg/AcpiView: Refactor DumpAcpiTableToFile
> 
> Method is refactored into two parts. A new method is created that dumps
> arbitrary buffers into a newly created file. This method is called from core 
> code
> after the core code determined the appropriate filename to be used.
> 
> This improves the modular design.
> 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
> Signed-off-by: Tomas Pilar 
> ---
>  .../UefiShellAcpiViewCommandLib/AcpiView.c| 38 +---
>  .../UefiShellAcpiViewCommandLib.c | 59 +++
>  .../UefiShellAcpiViewCommandLib.h | 17 +-
>  3 files changed, 76 insertions(+), 38 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> index 2507ec78204b..1b6721e6734f 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> @@ -27,8 +27,6 @@
>  #include "Arm/SbbrValidator.h"
>  #endif
> 
> -EFI_HII_HANDLE gShellAcpiViewHiiHandle = NULL;
> -
>  STATIC UINT32 mTableCount;
>  STATIC UINT32 mBinTableCount;
> 
> @@ -48,14 +46,10 @@ DumpAcpiTableToFile (
>IN CONST UINTN   Length
>)
>  {
> -  EFI_STATUS  Status;
>CHAR16  FileNameBuffer[MAX_FILE_NAME_LEN];
> -  SHELL_FILE_HANDLE   DumpFileHandle;
>UINTN   TransferBytes;
>SELECTED_ACPI_TABLE *SelectedTable;
> 
> -  DumpFileHandle = NULL;
> -  TransferBytes = Length;
>GetSelectedAcpiTable (&SelectedTable);
> 
>UnicodeSPrint (
> @@ -66,39 +60,9 @@ DumpAcpiTableToFile (
>  mBinTableCount++
>  );
> 
> -  Status = ShellOpenFileByName (
> - FileNameBuffer,
> - &DumpFileHandle,
> - EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE |
> EFI_FILE_MODE_CREATE,
> - 0
> - );
> -  if (EFI_ERROR (Status)) {
> -ShellPrintHiiEx (
> -  -1,
> -  -1,
> -  NULL,
> -  STRING_TOKEN (STR_GEN_READONLY_MEDIA),
> -  gShellAcpiViewHiiHandle,
> -  L"acpiview"
> -  );
> -return FALSE;
> -  }
> -
>Print (L"Dumping ACPI table to : %s ... ", FileNameBuffer);
> 
> -  Status = ShellWriteFile (
> - DumpFileHandle,
> - &TransferBytes,
> - (VOID*)Ptr
> - );
> -  if (EFI_ERROR (Status)) {
> -Print (L"ERROR: Failed to dump table to binary file.\n");
> -TransferBytes = 0;
> -  } else {
> -Print (L"DONE.\n");
> -  }
> -
> -  ShellCloseFile (&DumpFileHandle);
> +  TransferBytes = ShellDumpBufferToFile (FileNameBuffer, Ptr, Length);
>return (Length == TransferBytes);
>  }
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
> b.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
> b.c
> index c3942ad24e5b..e6a65d5bc5f7 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
> b.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> +++ andLib.c
> @@ -25,6 +25,7 @@
>  #include "UefiShellAcpiViewCommandLib.h"
> 
>  CONST CHAR16 gShellAcpiViewFileName[] = L"ShellCommand";
> +EFI_HII_HANDLE gShellAcpiViewHiiHandle = NULL;
> 
>  /**
>An array of acpiview command line parameters.
> @@ -98,6 +99,64 @@ RegisterAllParsers (
>return Status;
>  }
> 
> +/**
> +  Dump a buffer to a file. Print error message if a file cannot be created.
> +
> +  @param[in] FileName   The filename that shall be created to contain the 
> buffer.
> +  @param[in] Buffer Pointer to buffer that shall be dumped.
> +  @param[in] BufferSize The size of buffer to be dumped in bytes.
> +
> +  @return The number of bytes that were written **/ UINTN EFIAPI
> +ShellDumpBufferToFile (
> +  IN CONST CHAR16* FileNameBuffer,
> +  IN CONST VOID*   Buffer,
> +  IN CONST UINTN   BufferSize
> +  )
> +{
> +  EFI_STATUS  Status;
> +  SHELL_FILE_HANDLE   DumpFileHandle;
> +  UINTN   TransferBytes;
> +
> +  Status = ShellOpenFileByName (
> + FileNameBuffer,
> + &DumpFileHandle,
> + EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE |
> EFI_FILE_MODE_CREATE,
> + 0
> + );
> +
> +  if (EFI_ERROR (Status)) {
> +ShellPrintHiiEx (
> +  -1,
> +  -1,
> +  NULL,
> +  STRING_TOKEN (STR_GEN_READONLY_MEDIA),
> +  gShellAcpiViewHiiHandle,
> +  L"acpiview"
> +  );
> +return 0;
> +  }
> +
> +  TransferBytes = BufferSize;
> +  Status = ShellWriteFile (
> + DumpFileHandle,
> + &TransferBytes,
> + (VOID *) Buffer
> + );
> +
> +  if (EFI_ERROR (Status)) {
> +Print (L"ERROR: Failed to write binary file.\n");
> 

[edk2-devel] [PATCH v3 5/7] ShellPkg/AcpiView: Refactor DumpAcpiTableToFile

2020-06-15 Thread Tomas Pilar (tpilar)
Method is refactored into two parts. A new method is
created that dumps arbitrary buffers into a newly created
file. This method is called from core code after the core code
determined the appropriate filename to be used.

This improves the modular design.

Cc: Ray Ni 
Cc: Zhichao Gao 
Signed-off-by: Tomas Pilar 
---
 .../UefiShellAcpiViewCommandLib/AcpiView.c| 38 +---
 .../UefiShellAcpiViewCommandLib.c | 59 +++
 .../UefiShellAcpiViewCommandLib.h | 17 +-
 3 files changed, 76 insertions(+), 38 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
index 2507ec78204b..1b6721e6734f 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
@@ -27,8 +27,6 @@
 #include "Arm/SbbrValidator.h"
 #endif
 
-EFI_HII_HANDLE gShellAcpiViewHiiHandle = NULL;
-
 STATIC UINT32 mTableCount;
 STATIC UINT32 mBinTableCount;
 
@@ -48,14 +46,10 @@ DumpAcpiTableToFile (
   IN CONST UINTN   Length
   )
 {
-  EFI_STATUS  Status;
   CHAR16  FileNameBuffer[MAX_FILE_NAME_LEN];
-  SHELL_FILE_HANDLE   DumpFileHandle;
   UINTN   TransferBytes;
   SELECTED_ACPI_TABLE *SelectedTable;
 
-  DumpFileHandle = NULL;
-  TransferBytes = Length;
   GetSelectedAcpiTable (&SelectedTable);
 
   UnicodeSPrint (
@@ -66,39 +60,9 @@ DumpAcpiTableToFile (
 mBinTableCount++
 );
 
-  Status = ShellOpenFileByName (
- FileNameBuffer,
- &DumpFileHandle,
- EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE,
- 0
- );
-  if (EFI_ERROR (Status)) {
-ShellPrintHiiEx (
-  -1,
-  -1,
-  NULL,
-  STRING_TOKEN (STR_GEN_READONLY_MEDIA),
-  gShellAcpiViewHiiHandle,
-  L"acpiview"
-  );
-return FALSE;
-  }
-
   Print (L"Dumping ACPI table to : %s ... ", FileNameBuffer);
 
-  Status = ShellWriteFile (
- DumpFileHandle,
- &TransferBytes,
- (VOID*)Ptr
- );
-  if (EFI_ERROR (Status)) {
-Print (L"ERROR: Failed to dump table to binary file.\n");
-TransferBytes = 0;
-  } else {
-Print (L"DONE.\n");
-  }
-
-  ShellCloseFile (&DumpFileHandle);
+  TransferBytes = ShellDumpBufferToFile (FileNameBuffer, Ptr, Length);
   return (Length == TransferBytes);
 }
 
diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
index c3942ad24e5b..e6a65d5bc5f7 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
@@ -25,6 +25,7 @@
 #include "UefiShellAcpiViewCommandLib.h"
 
 CONST CHAR16 gShellAcpiViewFileName[] = L"ShellCommand";
+EFI_HII_HANDLE gShellAcpiViewHiiHandle = NULL;
 
 /**
   An array of acpiview command line parameters.
@@ -98,6 +99,64 @@ RegisterAllParsers (
   return Status;
 }
 
+/**
+  Dump a buffer to a file. Print error message if a file cannot be created.
+
+  @param[in] FileName   The filename that shall be created to contain the 
buffer.
+  @param[in] Buffer Pointer to buffer that shall be dumped.
+  @param[in] BufferSize The size of buffer to be dumped in bytes.
+
+  @return The number of bytes that were written
+**/
+UINTN
+EFIAPI
+ShellDumpBufferToFile (
+  IN CONST CHAR16* FileNameBuffer,
+  IN CONST VOID*   Buffer,
+  IN CONST UINTN   BufferSize
+  )
+{
+  EFI_STATUS  Status;
+  SHELL_FILE_HANDLE   DumpFileHandle;
+  UINTN   TransferBytes;
+
+  Status = ShellOpenFileByName (
+ FileNameBuffer,
+ &DumpFileHandle,
+ EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE,
+ 0
+ );
+
+  if (EFI_ERROR (Status)) {
+ShellPrintHiiEx (
+  -1,
+  -1,
+  NULL,
+  STRING_TOKEN (STR_GEN_READONLY_MEDIA),
+  gShellAcpiViewHiiHandle,
+  L"acpiview"
+  );
+return 0;
+  }
+
+  TransferBytes = BufferSize;
+  Status = ShellWriteFile (
+ DumpFileHandle,
+ &TransferBytes,
+ (VOID *) Buffer
+ );
+
+  if (EFI_ERROR (Status)) {
+Print (L"ERROR: Failed to write binary file.\n");
+TransferBytes = 0;
+  } else {
+Print (L"DONE.\n");
+  }
+
+  ShellCloseFile (&DumpFileHandle);
+  return TransferBytes;
+}
+
 /**
   Return the file name of the help text file if not using HII.
 
diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.h 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.h
index a3a29164004d..b1b1ffe63e28 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.h
@@ -8,7 +8,22 @@
 #