Re: [edk2-devel] [PATCH v5 3/6] ShellPkg/AcpiView: Update print-formatter prototype

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

From: "Sami Mujawar" <sami.mujawar@arm.com>
To: Rohit Mathew <Rohit.Mathew@arm.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: James Morse <James.Morse@arm.com>,
	Thomas Abraham <thomas.abraham@arm.com>,
	Zhichao Gao <zhichao.gao@intel.com>, nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v5 3/6] ShellPkg/AcpiView: Update print-formatter prototype
Date: Tue, 5 Dec 2023 11:48:29 +0000	[thread overview]
Message-ID: <E3B704DA-F954-452E-8FF5-E47181EC6D7A@arm.com> (raw)
In-Reply-To: <20231002171623.901196-1-Rohit.Mathew@arm.com>

Hi Rohit,

Thank you for this patch.
These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 02/10/2023, 18:16, "Rohit Mathew" <Rohit.Mathew@arm.com <mailto:Rohit.Mathew@arm.com>> wrote:


As of now, the print-formatter implemented by the FNPTR_PRINT_FORMATTER
function pointer takes two parameters, the format string and the pointer
to the field. For cases where the print-formatter has to have access to
the length of the field, there is no clean way to currently do it. In
order to resolve this, update the print-formatter's prototype to take
the length of the field as a third parameter. This change should improve
the overall robustness and flexibility of AcpiView.


Signed-off-by: Rohit Mathew <Rohit.Mathew@arm.com <mailto:Rohit.Mathew@arm.com>>
Cc: James Morse <james.Morse@arm.com <mailto:james.Morse@arm.com>>
Cc: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com>>
Cc: Thomas Abraham <thomas.abraham@arm.com <mailto:thomas.abraham@arm.com>>
Cc: Zhichao Gao <zhichao.gao@intel.com <mailto:zhichao.gao@intel.com>>
---
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 28 ++++++++++++++------
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 27 ++++++++++++++-----
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Aest/AestParser.c | 4 ++-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c | 8 ++++--
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c | 4 ++-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c | 4 ++-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 4 ++-
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 12 ++++++---
8 files changed, 67 insertions(+), 24 deletions(-)


diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 6823ba60cf..e0ad78250a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -319,12 +319,14 @@ DumpUint64 (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump3Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
Print (
@@ -343,12 +345,14 @@ Dump3Chars (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump4Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
Print (
@@ -368,12 +372,14 @@ Dump4Chars (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump6Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
Print (
@@ -395,12 +401,14 @@ Dump6Chars (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump8Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
Print (
@@ -424,12 +432,14 @@ Dump8Chars (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump12Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
Print (
@@ -587,7 +597,7 @@ ParseAcpi (
// the Format for printing
PrintFieldName (2, Parser[Index].NameStr);
if (Parser[Index].PrintFormatter != NULL) {
- Parser[Index].PrintFormatter (Parser[Index].Format, Ptr);
+ Parser[Index].PrintFormatter (Parser[Index].Format, Ptr, Parser[Index].Length);
} else if (Parser[Index].Format != NULL) {
switch (Parser[Index].Length) {
case 1:
@@ -681,12 +691,14 @@ DumpGasStruct (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
DumpGas (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
DumpGasStruct (Ptr, 2, sizeof (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE));
@@ -892,7 +904,7 @@ ParseAcpiBitFields (
// the Format for printing
PrintFieldName (2, Parser[Index].NameStr);
if (Parser[Index].PrintFormatter != NULL) {
- Parser[Index].PrintFormatter (Parser[Index].Format, (UINT8 *)&Data);
+ Parser[Index].PrintFormatter (Parser[Index].Format, (UINT8 *)&Data, Parser[Index].Length);
} else if (Parser[Index].Format != NULL) {
// convert bit length to byte length
switch ((Parser[Index].Length + 7) >> 3) {
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index 6d8b44d94a..e760467a9d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -130,12 +130,14 @@ DumpUint64 (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump3Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
);


/**
@@ -146,12 +148,14 @@ Dump3Chars (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump4Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
);


/**
@@ -162,12 +166,14 @@ Dump4Chars (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump6Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
);


/**
@@ -178,12 +184,14 @@ Dump6Chars (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump8Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
);


/**
@@ -194,12 +202,14 @@ Dump8Chars (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
Dump12Chars (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
);


/**
@@ -227,8 +237,9 @@ PrintFieldName (
@param [in] Format Format string for tracing the data as specified by
the 'Format' member of ACPI_PARSER.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
-typedef VOID (EFIAPI *FNPTR_PRINT_FORMATTER)(CONST CHAR16 *Format, UINT8 *Ptr);
+typedef VOID (EFIAPI *FNPTR_PRINT_FORMATTER)(CONST CHAR16 *Format, UINT8 *Ptr, UINT32 Length);


/**
This function pointer is the template for validating an ACPI table field.
@@ -469,12 +480,14 @@ DumpGasStruct (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
DumpGas (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
);


/**
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Aest/AestParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Aest/AestParser.c
index a37927b107..799b32cb2b 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Aest/AestParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Aest/AestParser.c
@@ -157,12 +157,14 @@ ValidateInterruptFlags (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
DumpVendorSpecificData (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
Print (
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c
index f9f08732ca..78bca0c248 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c
@@ -171,13 +171,15 @@ FormatByte (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the Action byte.
+ @param [in] Length Length of the field.
**/
STATIC
VOID
EFIAPI
DumpErstAction (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
FormatByte (ErstActionTable, *Ptr, ARRAY_SIZE (ErstActionTable));
@@ -188,13 +190,15 @@ DumpErstAction (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the Instruction byte.
+ @param [in] Length Length of the field.
**/
STATIC
VOID
EFIAPI
DumpErstInstruction (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
FormatByte (ErstInstructionTable, *Ptr, ARRAY_SIZE (ErstInstructionTable));
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
index f03b99ebde..2d6f10119d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
@@ -169,12 +169,14 @@ STATIC CONST ACPI_PARSER FadtFlagParser[] = {


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
DumpFadtFlags (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
if (Format != NULL) {
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c
index de8e9cf01f..0660c626dc 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.c
@@ -111,13 +111,15 @@ ValidateCacheAttributes (


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
STATIC
VOID
EFIAPI
DumpCacheAttributes (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
EFI_ACPI_6_4_HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATTRIBUTES *
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 3633329bea..f3dba486d7 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -223,12 +223,14 @@ STATIC CONST ACPI_PARSER LocalApicFlags[] = {


@param [in] Format Optional format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
VOID
EFIAPI
DumpLocalApicBitFlags (
IN CONST CHAR16 *Format OPTIONAL,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
if (Format != NULL) {
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
index 76534ccee3..c923a350d9 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
@@ -81,13 +81,15 @@ ValidateSratDeviceHandleType (


@param [in] Format Format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
STATIC
VOID
EFIAPI
DumpSratPciBdfNumber (
IN CONST CHAR16 *Format,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
CHAR16 Buffer[OUTPUT_FIELD_COLUMN_WIDTH];
@@ -169,13 +171,15 @@ STATIC CONST ACPI_PARSER SratDeviceHandlePciParser[] = {


@param [in] Format Format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
STATIC
VOID
EFIAPI
DumpSratDeviceHandle (
IN CONST CHAR16 *Format,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
if (SratDeviceHandleType == NULL) {
@@ -212,13 +216,15 @@ DumpSratDeviceHandle (


@param [in] Format Format string for tracing the data.
@param [in] Ptr Pointer to the start of the buffer.
+ @param [in] Length Length of the field.
**/
STATIC
VOID
EFIAPI
DumpSratApicProximity (
IN CONST CHAR16 *Format,
- IN UINT8 *Ptr
+ IN UINT8 *Ptr,
+ IN UINT32 Length
)
{
UINT32 ProximityDomain;
-- 
2.34.1







-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112076): https://edk2.groups.io/g/devel/message/112076
Mute This Topic: https://groups.io/mt/101716941/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

     prev parent reply	other threads:[~2023-12-05 11:48 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-02 17:16 [edk2-devel] [PATCH v5 3/6] ShellPkg/AcpiView: Update print-formatter prototype Rohit Mathew
2023-12-05 11:48 ` Sami Mujawar [this message]

find likely ancestor, descendant, or conflicting patches for this message:
( dfblob:6823ba60c dfblob:e0ad78250 dfblob:6d8b44d94 dfblob:e760467a9
dfblob:a37927b10 dfblob:799b32cb2 dfblob:f9f08732c dfblob:78bca0c24
dfblob:f03b99ebd dfblob:2d6f10119 dfblob:de8e9cf01 dfblob:0660c626d
dfblob:3633329be dfblob:f3dba486d dfblob:76534ccee dfblob:c923a350d )
 OR (
bs:"Re: [edk2-devel] [PATCH v5 3/6] ShellPkg/AcpiView: Update print-formatter prototype" )
	(help)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E3B704DA-F954-452E-8FF5-E47181EC6D7A@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox