[edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Smbios3Table used as SmbiosTable wrongly

2015-10-27 Thread Star Zeng 
This patch does below things.
1. Smbios3Table used as SmbiosTable wrongly after SmbiosTable got from 
configuration table.
2. Correct the return comments of entrypoint function.
3. Add parameters' comments for MeasureSmbiosTable().
4. Remove the tailing space from SmbiosMeasurementDxe.c.
5. Correct the Protocols and Guids usage comments in SmbiosMeasurementDxe.inf.
6. Add (VOID **)  typecast in LocateProtocol call for GCC build failure.
7. Use EFI_D_VERBOSE instead of EFI_D_INFO in InternalDumpData() and 
InternalDumpHex().
8. Use correct VendorGuid and VendorTable to measure.

Cc:Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.c| 75 +-
 .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf  |  6 +-
 2 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c 
b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
index 2152827..c10e9f7 100644
--- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
+++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
@@ -1,14 +1,14 @@
 /** @file
   This driver measures SMBIOS table to TPM.
-  
+
 Copyright (c) 2015, Intel Corporation. All rights reserved.
-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
-http://opensource.org/licenses/bsd-license.php 
   
-   
   
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,  
   
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.  
   
+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
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
@@ -125,7 +125,7 @@ InternalDumpData (
 {
   UINTN  Index;
   for (Index = 0; Index < Size; Index++) {
-DEBUG ((EFI_D_INFO, "%02x", (UINTN)Data[Index]));
+DEBUG ((EFI_D_VERBOSE, "%02x", (UINTN)Data[Index]));
   }
 }
 
@@ -152,15 +152,15 @@ InternalDumpHex (
   Count = Size / COLUME_SIZE;
   Left  = Size % COLUME_SIZE;
   for (Index = 0; Index < Count; Index++) {
-DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE));
+DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE));
 InternalDumpData (Data + Index * COLUME_SIZE, COLUME_SIZE);
-DEBUG ((EFI_D_INFO, "\n"));
+DEBUG ((EFI_D_VERBOSE, "\n"));
   }
 
   if (Left != 0) {
-DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE));
+DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE));
 InternalDumpData (Data + Index * COLUME_SIZE, Left);
-DEBUG ((EFI_D_INFO, "\n"));
+DEBUG ((EFI_D_VERBOSE, "\n"));
   }
 }
 
@@ -217,7 +217,7 @@ GetSmbiosStringById (
   // look for the two consecutive zeros, check the string limit by the way.
   //
   String = NULL;
-  while (*CharInStr != 0 || *(CharInStr+1) != 0) { 
+  while (*CharInStr != 0 || *(CharInStr+1) != 0) {
 if (*CharInStr == 0) {
   Size += 1;
   CharInStr++;
@@ -271,7 +271,7 @@ FilterSmbiosEntry (
   UINTN StringLen;
 
   DEBUG ((EFI_D_INFO, "Smbios Table (Type - %d):\n", ((SMBIOS_STRUCTURE 
*)TableEntry)->Type));
-  InternalDumpHex (TableEntry, TableEntrySize);
+  DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize););
 
   FilterStruct = GetFilterStructByType (((SMBIOS_STRUCTURE 
*)TableEntry)->Type);
   if (FilterStruct != NULL) {
@@ -297,7 +297,7 @@ FilterSmbiosEntry (
   }
 
   DEBUG ((EFI_D_INFO, "Filter Smbios Table (Type - %d):\n", ((SMBIOS_STRUCTURE 
*)TableEntry)->Type));
-  InternalDumpHex (TableEntry, TableEntrySize);
+  DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize););
 }
 
 /**
@@ -306,7 +306,7 @@ FilterSmbiosEntry (
 
   @param Head   Pointer to the beginning of SMBIOS structure.
   @param NumberOfStringsThe returned number of optional strings that 
follow the formatted structure.
-  
+
   @return Size  The returned size.
 **/
 UINTN
@@ -327,7 +327,7 @@ GetSmbiosStructureSize (
   //
   // look for the two consecutive zeros, check the string limit by the way.
   //
-  while (*CharInStr != 0 || *(CharInStr+1) != 0) { 
+  while (*CharInStr != 0 || *(CharInStr+1) != 0) {
 if (*CharInStr == 0) {
   Size += 1;
   CharInStr++;
@@ -432,7 +432,11 @@ FilterSmbiosTable (
 }
 
 /**
-  Measure SMBIOS with EV_EFI_HANDOFF_TABLES

Re: [edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Smbios3Table used as SmbiosTable wrongly

2015-10-27 Thread Ard Biesheuvel 
On 28 October 2015 at 10:33, Star Zeng  wrote:
> This patch does below things.
> 1. Smbios3Table used as SmbiosTable wrongly after SmbiosTable got from 
> configuration table.
> 2. Correct the return comments of entrypoint function.
> 3. Add parameters' comments for MeasureSmbiosTable().
> 4. Remove the tailing space from SmbiosMeasurementDxe.c.
> 5. Correct the Protocols and Guids usage comments in SmbiosMeasurementDxe.inf.
> 6. Add (VOID **)  typecast in LocateProtocol call for GCC build failure.
> 7. Use EFI_D_VERBOSE instead of EFI_D_INFO in InternalDumpData() and 
> InternalDumpHex().
> 8. Use correct VendorGuid and VendorTable to measure.
>

If you need a numbered list to summarize the changes you are making in
a patch, that is usually a strong hint that you should split it into
several patches. You have a bug fixes (1,8), comment fixes (2,3,5), a
functional change (7), a build fix (6) and a whitespace fix (4), so
you should split this into 5 or 6 patches at least.

-- 
Ard.



> Cc:Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.c| 75 
> +-
>  .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf  |  6 +-
>  2 files changed, 46 insertions(+), 35 deletions(-)
>
> diff --git 
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c 
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
> index 2152827..c10e9f7 100644
> --- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
> +++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
> @@ -1,14 +1,14 @@
>  /** @file
>This driver measures SMBIOS table to TPM.
> -
> +
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
> -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
> -http://opensource.org/licenses/bsd-license.php
> -
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +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
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
>  **/
>
> @@ -125,7 +125,7 @@ InternalDumpData (
>  {
>UINTN  Index;
>for (Index = 0; Index < Size; Index++) {
> -DEBUG ((EFI_D_INFO, "%02x", (UINTN)Data[Index]));
> +DEBUG ((EFI_D_VERBOSE, "%02x", (UINTN)Data[Index]));
>}
>  }
>
> @@ -152,15 +152,15 @@ InternalDumpHex (
>Count = Size / COLUME_SIZE;
>Left  = Size % COLUME_SIZE;
>for (Index = 0; Index < Count; Index++) {
> -DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE));
> +DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE));
>  InternalDumpData (Data + Index * COLUME_SIZE, COLUME_SIZE);
> -DEBUG ((EFI_D_INFO, "\n"));
> +DEBUG ((EFI_D_VERBOSE, "\n"));
>}
>
>if (Left != 0) {
> -DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE));
> +DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE));
>  InternalDumpData (Data + Index * COLUME_SIZE, Left);
> -DEBUG ((EFI_D_INFO, "\n"));
> +DEBUG ((EFI_D_VERBOSE, "\n"));
>}
>  }
>
> @@ -217,7 +217,7 @@ GetSmbiosStringById (
>// look for the two consecutive zeros, check the string limit by the way.
>//
>String = NULL;
> -  while (*CharInStr != 0 || *(CharInStr+1) != 0) {
> +  while (*CharInStr != 0 || *(CharInStr+1) != 0) {
>  if (*CharInStr == 0) {
>Size += 1;
>CharInStr++;
> @@ -271,7 +271,7 @@ FilterSmbiosEntry (
>UINTN StringLen;
>
>DEBUG ((EFI_D_INFO, "Smbios Table (Type - %d):\n", ((SMBIOS_STRUCTURE 
> *)TableEntry)->Type));
> -  InternalDumpHex (TableEntry, TableEntrySize);
> +  DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize););
>
>FilterStruct = GetFilterStructByType (((SMBIOS_STRUCTURE 
> *)TableEntry)->Type);
>if (FilterStruct != NULL) {
> @@ -297,7 +297,7 @@ FilterSmbiosEntry (
>}
>
>DEBUG ((EFI_D_INFO, "Filter Smbios Table (Type - %d):\n", 
> ((SMBIOS_STRUCTURE *)TableEntry)->Type));
> -  InternalDumpHex (TableEntry, TableEntrySize);
> +  DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize););
>  }
>
>  /**
> @@ -306,7 +306,7 @@ FilterSmbiosEntry (
>
>@param Head   Pointer to the beginning of SMBIOS structure.
>@param NumberOfStringsThe returned number of optional strings that 
> follow the formatted structure.
> -
> +
>@return Size  The returned size.
>  **/
>  UINTN
> @

Re: [edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Smbios3Table used as SmbiosTable wrongly

2015-10-27 Thread Yao, Jiewen 
Thanks.
Can we use CopyGuid() for below assignment?
HandoffTables.TableEntry[0].VendorGuid  = gEfiSmbiosTableGuid;

Reviewed-by: jiewen@intel.com


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star Zeng
Sent: Wednesday, October 28, 2015 9:33 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Smbios3Table used as 
SmbiosTable wrongly

This patch does below things.
1. Smbios3Table used as SmbiosTable wrongly after SmbiosTable got from 
configuration table.
2. Correct the return comments of entrypoint function.
3. Add parameters' comments for MeasureSmbiosTable().
4. Remove the tailing space from SmbiosMeasurementDxe.c.
5. Correct the Protocols and Guids usage comments in SmbiosMeasurementDxe.inf.
6. Add (VOID **)  typecast in LocateProtocol call for GCC build failure.
7. Use EFI_D_VERBOSE instead of EFI_D_INFO in InternalDumpData() and 
InternalDumpHex().
8. Use correct VendorGuid and VendorTable to measure.

Cc:Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.c| 75 +-
 .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf  |  6 +-
 2 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c 
b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
index 2152827..c10e9f7 100644
--- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
+++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
@@ -1,14 +1,14 @@
 /** @file
   This driver measures SMBIOS table to TPM.
-  
+
 Copyright (c) 2015, Intel Corporation. All rights reserved.
-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
-http://opensource.org/licenses/bsd-license.php 
   
-   
   
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,  
   
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.  
   
+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 http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
@@ -125,7 +125,7 @@ InternalDumpData (
 {
   UINTN  Index;
   for (Index = 0; Index < Size; Index++) {
-DEBUG ((EFI_D_INFO, "%02x", (UINTN)Data[Index]));
+DEBUG ((EFI_D_VERBOSE, "%02x", (UINTN)Data[Index]));
   }
 }
 
@@ -152,15 +152,15 @@ InternalDumpHex (
   Count = Size / COLUME_SIZE;
   Left  = Size % COLUME_SIZE;
   for (Index = 0; Index < Count; Index++) {
-DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE));
+DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE));
 InternalDumpData (Data + Index * COLUME_SIZE, COLUME_SIZE);
-DEBUG ((EFI_D_INFO, "\n"));
+DEBUG ((EFI_D_VERBOSE, "\n"));
   }
 
   if (Left != 0) {
-DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE));
+DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE));
 InternalDumpData (Data + Index * COLUME_SIZE, Left);
-DEBUG ((EFI_D_INFO, "\n"));
+DEBUG ((EFI_D_VERBOSE, "\n"));
   }
 }
 
@@ -217,7 +217,7 @@ GetSmbiosStringById (
   // look for the two consecutive zeros, check the string limit by the way.
   //
   String = NULL;
-  while (*CharInStr != 0 || *(CharInStr+1) != 0) { 
+  while (*CharInStr != 0 || *(CharInStr+1) != 0) {
 if (*CharInStr == 0) {
   Size += 1;
   CharInStr++;
@@ -271,7 +271,7 @@ FilterSmbiosEntry (
   UINTN StringLen;
 
   DEBUG ((EFI_D_INFO, "Smbios Table (Type - %d):\n", ((SMBIOS_STRUCTURE 
*)TableEntry)->Type));
-  InternalDumpHex (TableEntry, TableEntrySize);
+  DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize););
 
   FilterStruct = GetFilterStructByType (((SMBIOS_STRUCTURE 
*)TableEntry)->Type);
   if (FilterStruct != NULL) {
@@ -297,7 +297,7 @@ FilterSmbiosEntry (
   }
 
   DEBUG ((EFI_D_INFO, "Filter Smbios Table (Type - %d):\n", ((SMBIOS_STRUCTURE 
*)TableEntry)->Type));
-  InternalDumpHex (TableEntry, TableEntrySize);
+  DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize););
 }
 
 /**
@@ -306,7 +306,7 @@ FilterSmbiosEntry (
 
   @param Head   Pointer to the beg

Re: [edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Smbios3Table used as SmbiosTable wrongly

2015-10-27 Thread Zeng, Star 

On 2015/10/28 9:44, Ard Biesheuvel wrote:

On 28 October 2015 at 10:33, Star Zeng  wrote:

This patch does below things.
1. Smbios3Table used as SmbiosTable wrongly after SmbiosTable got from 
configuration table.
2. Correct the return comments of entrypoint function.
3. Add parameters' comments for MeasureSmbiosTable().
4. Remove the tailing space from SmbiosMeasurementDxe.c.
5. Correct the Protocols and Guids usage comments in SmbiosMeasurementDxe.inf.
6. Add (VOID **)  typecast in LocateProtocol call for GCC build failure.
7. Use EFI_D_VERBOSE instead of EFI_D_INFO in InternalDumpData() and 
InternalDumpHex().
8. Use correct VendorGuid and VendorTable to measure.



If you need a numbered list to summarize the changes you are making in
a patch, that is usually a strong hint that you should split it into
several patches. You have a bug fixes (1,8), comment fixes (2,3,5), a
functional change (7), a build fix (6) and a whitespace fix (4), so
you should split this into 5 or 6 patches at least.


Good comments.
Just followed your suggestion to resend the split patch set.

Thanks,
Star





___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Smbios3Table used as SmbiosTable wrongly

2015-10-27 Thread Zeng, Star 

On 2015/10/28 10:58, Yao, Jiewen wrote:

Thanks.
Can we use CopyGuid() for below assignment?
HandoffTables.TableEntry[0].VendorGuid  = gEfiSmbiosTableGuid;


Yes, have included this into PATCH 1/5 of the new split patch set and 
with your Reviewed-by.


Thanks,
Star



Reviewed-by: jiewen@intel.com


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star Zeng
Sent: Wednesday, October 28, 2015 9:33 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Smbios3Table used as 
SmbiosTable wrongly

This patch does below things.
1. Smbios3Table used as SmbiosTable wrongly after SmbiosTable got from 
configuration table.
2. Correct the return comments of entrypoint function.
3. Add parameters' comments for MeasureSmbiosTable().
4. Remove the tailing space from SmbiosMeasurementDxe.c.
5. Correct the Protocols and Guids usage comments in SmbiosMeasurementDxe.inf.
6. Add (VOID **)  typecast in LocateProtocol call for GCC build failure.
7. Use EFI_D_VERBOSE instead of EFI_D_INFO in InternalDumpData() and 
InternalDumpHex().
8. Use correct VendorGuid and VendorTable to measure.

Cc:Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
  .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.c| 75 +-
  .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf  |  6 +-
  2 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c 
b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
index 2152827..c10e9f7 100644
--- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
+++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
@@ -1,14 +1,14 @@
  /** @file
This driver measures SMBIOS table to TPM.
-
+
  Copyright (c) 2015, Intel Corporation. All rights reserved.
-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
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+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 http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.

  **/

@@ -125,7 +125,7 @@ InternalDumpData (
  {
UINTN  Index;
for (Index = 0; Index < Size; Index++) {
-DEBUG ((EFI_D_INFO, "%02x", (UINTN)Data[Index]));
+DEBUG ((EFI_D_VERBOSE, "%02x", (UINTN)Data[Index]));
}
  }

@@ -152,15 +152,15 @@ InternalDumpHex (
Count = Size / COLUME_SIZE;
Left  = Size % COLUME_SIZE;
for (Index = 0; Index < Count; Index++) {
-DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE));
+DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE));
  InternalDumpData (Data + Index * COLUME_SIZE, COLUME_SIZE);
-DEBUG ((EFI_D_INFO, "\n"));
+DEBUG ((EFI_D_VERBOSE, "\n"));
}

if (Left != 0) {
-DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE));
+DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE));
  InternalDumpData (Data + Index * COLUME_SIZE, Left);
-DEBUG ((EFI_D_INFO, "\n"));
+DEBUG ((EFI_D_VERBOSE, "\n"));
}
  }

@@ -217,7 +217,7 @@ GetSmbiosStringById (
// look for the two consecutive zeros, check the string limit by the way.
//
String = NULL;
-  while (*CharInStr != 0 || *(CharInStr+1) != 0) {
+  while (*CharInStr != 0 || *(CharInStr+1) != 0) {
  if (*CharInStr == 0) {
Size += 1;
CharInStr++;
@@ -271,7 +271,7 @@ FilterSmbiosEntry (
UINTN StringLen;

DEBUG ((EFI_D_INFO, "Smbios Table (Type - %d):\n", ((SMBIOS_STRUCTURE 
*)TableEntry)->Type));
-  InternalDumpHex (TableEntry, TableEntrySize);
+  DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize););

FilterStruct = GetFilterStructByType (((SMBIOS_STRUCTURE 
*)TableEntry)->Type);
if (FilterStruct != NULL) {
@@ -297,7 +297,7 @@ FilterSmbiosEntry (
}

DEBUG ((EFI_D_INFO, "Filter Smbios Table (Type - %d):\n", ((SMBIOS_STRUCTURE 
*)TableEntry)->Type));
-  InternalDumpHex (TableEntry, TableEntrySize);
+  DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize););
  }

  /**
@@ -306,7 +306,7 @@ FilterSmbiosEntry (

@param Head   Pointer to the beginning of SMBIOS structure.
@param NumberOfStr

Re: [edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Smbios3Table used as SmbiosTable wrongly

2015-10-27 Thread Ard Biesheuvel 
On 28 October 2015 at 13:26, Zeng, Star  wrote:
> On 2015/10/28 9:44, Ard Biesheuvel wrote:
>>
>> On 28 October 2015 at 10:33, Star Zeng  wrote:
>>>
>>> This patch does below things.
>>> 1. Smbios3Table used as SmbiosTable wrongly after SmbiosTable got from
>>> configuration table.
>>> 2. Correct the return comments of entrypoint function.
>>> 3. Add parameters' comments for MeasureSmbiosTable().
>>> 4. Remove the tailing space from SmbiosMeasurementDxe.c.
>>> 5. Correct the Protocols and Guids usage comments in
>>> SmbiosMeasurementDxe.inf.
>>> 6. Add (VOID **)  typecast in LocateProtocol call for GCC build failure.
>>> 7. Use EFI_D_VERBOSE instead of EFI_D_INFO in InternalDumpData() and
>>> InternalDumpHex().
>>> 8. Use correct VendorGuid and VendorTable to measure.
>>>
>>
>> If you need a numbered list to summarize the changes you are making in
>> a patch, that is usually a strong hint that you should split it into
>> several patches. You have a bug fixes (1,8), comment fixes (2,3,5), a
>> functional change (7), a build fix (6) and a whitespace fix (4), so
>> you should split this into 5 or 6 patches at least.
>
>
> Good comments.
> Just followed your suggestion to resend the split patch set.
>

Thank you, Star!
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel