From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 32FB0780091 for ; Tue, 10 Oct 2023 10:13:56 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=Z2Y7o4NkC2LxEivsywX/ZzkncVmuMqqmjfIXWBXK34E=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:From:Subject:To:Cc:References:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1696932834; v=1; b=gVjB4SoM3In9aK+nTwli6c8YhsHXjSnoCpp3AKGfu1o9fEp77pGWLbC9h42IVsiZIZQX6BUv wyRvrYobg03/hue/YIMOzmeIF4UB4IIlILm9kvAJhFb3659+Fyvd+CQpA+R51MXIkRItSQANb3z 9tQ8EI4ybrPdDaaU1azFt5e8= X-Received: by 127.0.0.2 with SMTP id kh4bYY7687511xjPa77cWJma; Tue, 10 Oct 2023 03:13:54 -0700 X-Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.87894.1696932834171424675 for ; Tue, 10 Oct 2023 03:13:54 -0700 X-Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 796EA1FB; Tue, 10 Oct 2023 03:14:34 -0700 (PDT) X-Received: from [10.34.100.114] (e126645.nice.arm.com [10.34.100.114]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 45D223F762; Tue, 10 Oct 2023 03:13:53 -0700 (PDT) Message-ID: Date: Tue, 10 Oct 2023 12:13:53 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: "PierreGondois" Subject: Re: [edk2-devel] [PATCH 1/2] DynamicTablesPkg/TableHelperLib: Fix and improve text handling To: Jeshua Smith , devel@edk2.groups.io Cc: Sami.Mujawar@arm.com References: In-Reply-To: Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,pierre.gondois@arm.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: dHexwsb6XAp1TZD8q6ntSr4Lx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=gVjB4SoM; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=arm.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io Hello Jeshua, Just a small remark, but this should be equivalent, so: Reviewed-by: Pierre Gondois On 10/6/23 18:28, Jeshua Smith wrote: > This fixes two bugs and adds some enhancements to the handling of > characters and strings in objects being printed by the CM ObjectParser. >=20 > Bug fixes: > 1. PrintOemID() currently attempts to print characters with "%C", > but the correct syntax is (lowercase) "%c". This bug results in > "CCCCCC" being printed instead of the actual ASCII characters. > 2. PrintString() is being passed a pointer to data in objects, but in > some cases this data is the actual string to print and other cases > it is a pointer to the string to print. This adds a PrintStringPtr > function and uses the correct functions depending on the situation. >=20 > Enhancements: > 1. Some objects contain ASCII characters, which are currently printed > as their hex values. This adds functions to print out ASCII > character fields as text rather than hex, and uses those functions in > several cases where the object data is defined to be ASCII. > 2. The PrintOemID() function is replaced with the new identical but more > generecically-named PrintChar6() function. >=20 > Signed-off-by: Jeshua Smith > --- > .../ConfigurationManagerObjectParser.c | 176 ++++++++++++++---- > 1 file changed, 143 insertions(+), 33 deletions(-) >=20 > diff --git a/DynamicTablesPkg/Library/Common/TableHelperLib/Configuration= ManagerObjectParser.c b/DynamicTablesPkg/Library/Common/TableHelperLib/Conf= igurationManagerObjectParser.c > index 99d6032510..92df1efee8 100644 > --- a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManager= ObjectParser.c > +++ b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManager= ObjectParser.c > @@ -14,7 +14,7 @@ > STATIC > VOID > EFIAPI > -PrintOemId ( > +PrintString ( > CONST CHAR8 *Format, > UINT8 *Ptr > ); > @@ -22,7 +22,31 @@ PrintOemId ( > STATIC > VOID > EFIAPI > -PrintString ( > +PrintStringPtr ( > + CONST CHAR8 *Format, > + UINT8 *Ptr > + ); > + > +STATIC > +VOID > +EFIAPI > +PrintChar4 ( > + CONST CHAR8 *Format, > + UINT8 *Ptr > + ); > + > +STATIC > +VOID > +EFIAPI > +PrintChar6 ( > + CONST CHAR8 *Format, > + UINT8 *Ptr > + ); > + > +STATIC > +VOID > +EFIAPI > +PrintChar8 ( > CONST CHAR8 *Format, > UINT8 *Ptr > ); > @@ -190,16 +214,16 @@ STATIC CONST CM_OBJ_PARSER CmArmItsGroupNodeParser= [] =3D { > /** A parser for EArmObjNamedComponent. > */ > STATIC CONST CM_OBJ_PARSER CmArmNamedComponentNodeParser[] =3D { > - { "Token", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }= , > - { "IdMappingCount", 4, "0x%x", NULL }= , > - { "IdMappingToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL }= , > - { "Flags", 4, "0x%x", NULL }= , > - { "CacheCoherent", 4, "0x%x", NULL }= , > - { "AllocationHints", 1, "0x%x", NULL }= , > - { "MemoryAccessFlags", 1, "0x%x", NULL }= , > - { "AddressSizeLimit", 1, "0x%x", NULL }= , > - { "ObjectName", sizeof (CHAR8 *), NULL, PrintString }= , > - { "Identifier", 4, "0x%x", NULL }= , > + { "Token", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL = }, > + { "IdMappingCount", 4, "0x%x", NULL = }, > + { "IdMappingToken", sizeof (CM_OBJECT_TOKEN), "0x%p", NULL = }, > + { "Flags", 4, "0x%x", NULL = }, > + { "CacheCoherent", 4, "0x%x", NULL = }, > + { "AllocationHints", 1, "0x%x", NULL = }, > + { "MemoryAccessFlags", 1, "0x%x", NULL = }, > + { "AddressSizeLimit", 1, "0x%x", NULL = }, > + { "ObjectName", sizeof (CHAR8 *), NULL, PrintStringPt= r }, > + { "Identifier", 4, "0x%x", NULL = }, > }; > =20 > /** A parser for EArmObjRootComplex. > @@ -740,19 +764,19 @@ STATIC CONST CM_OBJ_PARSER_ARRAY ArmNamespaceObjec= tParser[] =3D { > */ > STATIC CONST CM_OBJ_PARSER StdObjCfgMgrInfoParser[] =3D { > { "Revision", 4, "0x%x", NULL }, > - { "OemId[6]", 6, "%C%C%C%C%C%C", PrintOemId } > + { "OemId[6]", 6, "%c%c%c%c%c%c", PrintChar6 } NIT: Is it necessary, since "%c%c%c%c%c%c" is the default format of PrintChar6()= ? > }; > =20 > /** A parser for EStdObjAcpiTableList. > */ > STATIC CONST CM_OBJ_PARSER StdObjAcpiTableInfoParser[] =3D { > - { "AcpiTableSignature", 4, "0x%x"= , NULL }, > - { "AcpiTableRevision", 1, "%d", = NULL }, > - { "TableGeneratorId", sizeof (ACPI_TABLE_GENERATOR_ID), "0x%x"= , NULL }, > - { "AcpiTableData", sizeof (EFI_ACPI_DESCRIPTION_HEADER *), "0x%p"= , NULL }, > - { "OemTableId", 8, "0x%LL= X", NULL }, > - { "OemRevision", 4, "0x%x"= , NULL }, > - { "MinorRevision", 1, "0x%x"= , NULL }, > + { "AcpiTableSignature", 4, "%c%c%= c%c", PrintChar4 }, > + { "AcpiTableRevision", 1, "%d", = NULL }, > + { "TableGeneratorId", sizeof (ACPI_TABLE_GENERATOR_ID), "0x%x"= , NULL }, > + { "AcpiTableData", sizeof (EFI_ACPI_DESCRIPTION_HEADER *), "0x%p"= , NULL }, > + { "OemTableId", 8, "%c%c%= c%c%c%c%c%c", PrintChar8 }, > + { "OemRevision", 4, "0x%x"= , NULL }, > + { "MinorRevision", 1, "0x%x"= , NULL }, > }; > =20 > /** A parser for EStdObjSmbiosTableList. > @@ -773,22 +797,99 @@ STATIC CONST CM_OBJ_PARSER_ARRAY StdNamespaceObjec= tParser[] =3D { > ARRAY_SIZE (StdObjSmbiosTableInfoParser) }, > }; > =20 > -/** Print OEM Id. > +/** Print string data. > + > + The string must be NULL terminated. > + > + @param [in] Format Format to print the Ptr. > + @param [in] Ptr Pointer to the string. > +**/ > +STATIC > +VOID > +EFIAPI > +PrintString ( > + IN CONST CHAR8 *Format, > + IN UINT8 *Ptr > + ) > +{ > + if (Ptr =3D=3D NULL) { > + ASSERT (0); > + return; > + } > + > + DEBUG ((DEBUG_ERROR, "%a", Ptr)); > +} > + > +/** Print string from pointer. > + > + The string must be NULL terminated. > + > + @param [in] Format Format to print the string. > + @param [in] Ptr Pointer to the string pointer. > +**/ > +STATIC > +VOID > +EFIAPI > +PrintStringPtr ( > + IN CONST CHAR8 *Format, > + IN UINT8 *Ptr > + ) > +{ > + UINT8 *String; > + > + if (Ptr =3D=3D NULL) { > + ASSERT (0); > + return; > + } > + > + String =3D *(UINT8 **)Ptr; > + > + if (String =3D=3D NULL) { > + String =3D (UINT8 *)"(NULLPTR)"; > + } > + > + PrintString (Format, String); > +} > + > +/** Print 4 characters. > =20 > @param [in] Format Format to print the Ptr. > - @param [in] Ptr Pointer to the OEM Id. > + @param [in] Ptr Pointer to the characters. > **/ > STATIC > VOID > EFIAPI > -PrintOemId ( > +PrintChar4 ( > IN CONST CHAR8 *Format, > IN UINT8 *Ptr > ) > { > DEBUG (( > - DEBUG_INFO, > - (Format !=3D NULL) ? Format : "%C%C%C%C%C%C", > + DEBUG_ERROR, > + (Format !=3D NULL) ? Format : "%c%c%c%c", > + Ptr[0], > + Ptr[1], > + Ptr[2], > + Ptr[3] > + )); > +} > + > +/** Print 6 characters. > + > + @param [in] Format Format to print the Ptr. > + @param [in] Ptr Pointer to the characters. > +**/ > +STATIC > +VOID > +EFIAPI > +PrintChar6 ( > + IN CONST CHAR8 *Format, > + IN UINT8 *Ptr > + ) > +{ > + DEBUG (( > + DEBUG_ERROR, > + (Format !=3D NULL) ? Format : "%c%c%c%c%c%c", > Ptr[0], > Ptr[1], > Ptr[2], > @@ -798,22 +899,31 @@ PrintOemId ( > )); > } > =20 > -/** Print string. > - > - The string must be NULL terminated. > +/** Print 8 characters. > =20 > @param [in] Format Format to print the Ptr. > - @param [in] Ptr Pointer to the string. > + @param [in] Ptr Pointer to the characters. > **/ > STATIC > VOID > EFIAPI > -PrintString ( > - CONST CHAR8 *Format, > - UINT8 *Ptr > +PrintChar8 ( > + IN CONST CHAR8 *Format, > + IN UINT8 *Ptr > ) > { > - DEBUG ((DEBUG_ERROR, "%a", Ptr)); > + DEBUG (( > + DEBUG_ERROR, > + (Format !=3D NULL) ? Format : "%c%c%c%c%c%c%c%c", > + Ptr[0], > + Ptr[1], > + Ptr[2], > + Ptr[3], > + Ptr[4], > + Ptr[5], > + Ptr[6], > + Ptr[7] > + )); > } > =20 > /** Print fields of the objects. -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109487): https://edk2.groups.io/g/devel/message/109487 Mute This Topic: https://groups.io/mt/101801390/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-