From: "Girish Mahadevan" <gmahadevan@nvidia.com>
To: Sami Mujawar <sami.mujawar@arm.com>, devel@edk2.groups.io
Cc: Alexei.Fedorov@arm.com, pierre.gondois@arm.com,
jbrasen@nvidia.com, ashishsingha@nvidia.com, nramirez@nvidia.com,
wwatson@nvidia.com, Samer.El-Haj-Mahmoud@arm.com,
Akanksha.Jain2@arm.com, Matteo.Carlini@arm.com,
Ben.Adderson@arm.com, nd@arm.com
Subject: Re: [PATCH v1 1/1] DynamicTablesPkg: Add SMBIOS String table helper library
Date: Tue, 4 Oct 2022 08:38:51 -0600 [thread overview]
Message-ID: <cd6246bf-3d24-a5eb-386d-6bc5b5dfd14e@nvidia.com> (raw)
In-Reply-To: <d12e8dea-acd3-6193-9b89-5128008df462@arm.com>
Hi Sami
Response inline.[GM]
Best Regards
Girish
On 10/4/2022 2:16 AM, Sami Mujawar wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Girish,
>
> There are 2 cases that need handling. Please see my response inline
> marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> On 04/10/2022 04:01 am, Girish Mahadevan wrote:
>> Hello Sami,
>>
>> My apologies for the late response. I had one question/comment.
>>
>> Comment marked with [GM] inline.
>>
>> Best Regards
>> Girish
>>
>> On 9/12/2022 8:18 AM, Sami Mujawar wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> The Section 6.1.3, SMBIOS specification version 3.6.0 describes the
>>> handling of test strings in SMBIOS tables.
>>>
>>> Test strings are added at the end of the formatted portion of the SMBIOS
>>> structure and are referenced by index in the SMBIOS structure.
>>>
>>> Therefore, introduce a SmbiosStringTableLib to simplify the publishing
>>> of the string set.
>>>
>>> SmbiosStringTableLib introduces a concept of string table which records
>>> the references to the SMBIOS strings as they are added and returns an
>>> string reference which is then assigned to the string field in the
>>> formatted portion of the SMBIOS structure. Once all strings are added,
>>> the library provides an interface to get the required size for the
>>> string
>>> set. This allows sufficient memory to be allocated for the SMBIOS table.
>>> The library also provides an interface to publish the string set in
>>> accordance with the SMBIOS specification.
>>>
>>> Example:
>>> EFI_STATUS
>>> BuildSmbiosType17Table () {
>>> STRING_TABLE StrTable;
>>> UINT8 DevLocatorRef;
>>> UINT8 BankLocatorRef;
>>> SMBIOS_TABLE_TYPE17 *SmbiosRecord;
>>> CHAR8 *StringSet;
>>> ...
>>>
>>> // Initialize string table for 7 strings
>>> StringTableInitialize (&StrTable, 7);
>>>
>>> StringTableAddString (&StrTable, "SIMM 3", &DevLocatorRef);
>>> StringTableAddString (&StrTable, "Bank 0", &BankLocatorRef);
>>> ...
>>>
>>> SmbiosRecord = AllocateZeroPool (
>>> sizeof (SMBIOS_TABLE_TYPE17) +
>>> StringTableGetStringSetSize (&StrTable)
>>> );
>>> ...
>>> SmbiosRecord->DeviceLocator = DevLocatorRef;
>>> SmbiosRecord->BankLocator = BankLocatorRef;
>>> ...
>>> // get the string set area
>>> StringSet = (CHAR8*)(SmbiosRecord + 1);
>>>
>>> // publish the string set
>>> StringTablePublishStringSet (
>>> &StrTable,
>>> StringSet,
>>> StringTableGetStringSetSize (&StrTable)
>>> );
>>>
>>> // free string table
>>> StringTableFree (&StrTable);
>>>
>>> return EFI_SUCCESS;
>>> }
>>>
>>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>>> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>>> Cc: Pierre Gondois <pierre.gondois@arm.com>
>>> Cc: Girish Mahadevan <gmahadevan@nvidia.com>
>>> Cc: Jeff Brasen <jbrasen@nvidia.com>
>>> Cc: Ashish Singhal <ashishsingha@nvidia.com>
>>> Cc: Nick Ramirez <nramirez@nvidia.com>
>>> Cc: William Watson <wwatson@nvidia.com>
>>> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>>> ---
>>> The changes can be seen at:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsamimujawar%2Fedk2%2Ftree%2F2370_smbios_stringlib_v1&data=05%7C01%7Cgmahadevan%40nvidia.com%7Cc983eff139e143b459ea08daa5e0c308%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638004682041794981%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=S5QKosbQutvQs17jTnZUHrxrTcxJmClwuUpdMRVoJKU%3D&reserved=0
>>>
>>> DynamicTablesPkg/DynamicTables.dsc.inc | 3 +-
>>> DynamicTablesPkg/DynamicTablesPkg.dec | 5 +-
>>> DynamicTablesPkg/DynamicTablesPkg.dsc | 3 +-
>>> DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h | 119 ++++++++++
>>> DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
>>> | 227 ++++++++++++++++++++
>>> DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
>>> | 25 +++
>>> 6 files changed, 379 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc
>>> b/DynamicTablesPkg/DynamicTables.dsc.inc
>>> index
>>> 3d4fa0c4c4b67d6770aee8705c80cc18d20c823a..d35acc1788f2ddc5e2b5fc3e4bedfd48251f7ec8
>>> 100644
>>> --- a/DynamicTablesPkg/DynamicTables.dsc.inc
>>> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
>>> @@ -1,7 +1,7 @@
>>> ## @file
>>> # Dsc include file for Dynamic Tables Framework.
>>> #
>>> -# Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
>>> +# Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
>>> #
>>> # SPDX-License-Identifier: BSD-2-Clause-Patent
>>> #
>>> @@ -18,6 +18,7 @@ [LibraryClasses.common]
>>> SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>>> SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
>>> TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
>>> +
>>> SmbiosStringTableLib|DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
>>>
>>> [Components.common]
>>> #
>>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec
>>> b/DynamicTablesPkg/DynamicTablesPkg.dec
>>> index
>>> cc34c2bdd6ff9b5ca508961b3d0fe85ffbb73c12..2a79cfd4edebbdff05bee66fa01a17d68252e8a7
>>> 100644
>>> --- a/DynamicTablesPkg/DynamicTablesPkg.dec
>>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dec
>>> @@ -1,7 +1,7 @@
>>> ## @file
>>> # dec file for Dynamic Tables Framework.
>>> #
>>> -# Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
>>> +# Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
>>> #
>>> # SPDX-License-Identifier: BSD-2-Clause-Patent
>>> #
>>> @@ -39,6 +39,9 @@ [LibraryClasses]
>>> ## @libraryclass Defines a set of helper methods.
>>> TableHelperLib|Include/Library/TableHelperLib.h
>>>
>>> + ## @libraryclass Defines a set of SMBIOS string helper methods.
>>> + SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
>>> +
>>> [Protocols]
>>> # Configuration Manager Protocol GUID
>>> gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82,
>>> 0x4894, { 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
>>> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc
>>> b/DynamicTablesPkg/DynamicTablesPkg.dsc
>>> index
>>> 07cc837552f587fe5bf9031e0061b0234e8698d4..bd5084a9008f040acdd16200ae8cdb23455ac101
>>> 100644
>>> --- a/DynamicTablesPkg/DynamicTablesPkg.dsc
>>> +++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
>>> @@ -2,7 +2,7 @@
>>> # Dsc file for Dynamic Tables Framework.
>>> #
>>> # Copyright (c) 2019, Linaro Limited. All rights reserved.<BR>
>>> -# Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
>>> +# Copyright (c) 2019 - 2022, Arm Limited. All rights reserved.<BR>
>>> #
>>> # SPDX-License-Identifier: BSD-2-Clause-Patent
>>> #
>>> @@ -46,6 +46,7 @@ [Components.common]
>>> DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
>>> DynamicTablesPkg/Library/FdtHwInfoParserLib/FdtHwInfoParserLib.inf
>>> DynamicTablesPkg/Library/Common/DynamicPlatRepoLib/DynamicPlatRepoLib.inf
>>> +
>>> DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
>>>
>>> [BuildOptions]
>>> *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
>>> diff --git a/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
>>> b/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
>>> new file mode 100644
>>> index
>>> 0000000000000000000000000000000000000000..246d4d30ddf901640ea720c108e2971552ec6c96
>>> --- /dev/null
>>> +++ b/DynamicTablesPkg/Include/Library/SmbiosStringTableLib.h
>>> @@ -0,0 +1,119 @@
>>> +/** @file
>>> + SMBIOS String Table Helper library.
>>> +
>>> + Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
>>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +**/
>>> +
>>> +#ifndef SMBIOS_STRING_TABLE_H_
>>> +#define SMBIOS_STRING_TABLE_H_
>>> +
>>> +/** A structure representing a string in the string table.
>>> +*/
>>> +typedef struct StringElement {
>>> + /// Length of the string (does not include the NULL termination)
>>> + UINTN StringLen;
>>> +
>>> + /// Reference to the string
>>> + CONST CHAR8 *String;
>>> +} STRING_ELEMENT;
>>> +
>>> +/** A structure representing a string table.
>>> +*/
>>> +typedef struct StringTable {
>>> + /// Count of strings in the table
>>> + UINT8 StrCount;
>>> +
>>> + /// Total length of all strings in the table (does not include
>>> + // the NULL termination for each string)
>>> + UINTN TotalStrLen;
>>> +
>>> + /// Maximum string count
>>> + UINT8 MaxStringElements;
>>> +
>>> + /// Pointer to the string table elements
>>> + STRING_ELEMENT *Elements;
>>> +} STRING_TABLE;
>>> +
>>> +/** Add a string to the string table
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> + @param [IN] Str Pointer to the string
>>> + @param [OUT] StrRef Optional pointer to retrieve the string field
>>> + reference of the string in the string table
>>> +
>>> + @return EFI_SUCCESS Success
>>> + @return EFI_INVALID_PARAMETER Invalid string table pointer
>>> + @return EFI_BUFFER_TOO_SMALL Insufficient space to add string
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +StringTableAddString (
>>> + IN STRING_TABLE *CONST StrTable,
>>> + IN CONST CHAR8 *Str,
>>> + OUT UINT8 *StrRef OPTIONAL
>>> + );
>>> +
>>> +/** Returns the total size required to publish the strings to the
>>> SMBIOS
>>> + string area.
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> +
>>> + @return Total size required to publish the strings in the SMBIOS
>>> string area.
>>> +**/
>>> +UINTN
>>> +EFIAPI
>>> +StringTableGetStringSetSize (
>>> + IN STRING_TABLE *CONST StrTable
>>> + );
>>> +
>>> +/** Iterate through the string table and publish the strings in the
>>> SMBIOS
>>> + string area.
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> + @param [IN] SmbiosStringAreaStart Start address of the SMBIOS
>>> string area.
>>> + @param [IN] SmbiosStringAreaSize Size of the SMBIOS string area.
>>> +
>>> + @return EFI_SUCCESS Success
>>> + @return EFI_INVALID_PARAMETER Invalid string table pointer
>>> + @return EFI_BUFFER_TOO_SMALL Insufficient space to publish strings
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +StringTablePublishStringSet (
>>> + IN STRING_TABLE *CONST StrTable,
>>> + IN CHAR8 *CONST SmbiosStringAreaStart,
>>> + IN CONST UINTN SmbiosStringAreaSize
>>> + );
>>> +
>>> +/** Initialise the string table and allocate memory for the string
>>> elements.
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> + @param [IN] MaxStringElements Maximum number of strings that the
>>> string
>>> + table can hold.
>>> +
>>> + @return EFI_SUCCESS Success
>>> + @return EFI_INVALID_PARAMETER Invalid string table pointer
>>> + @return EFI_OUT_OF_RESOURCES Failed to allocate memory for
>>> string elements
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +StringTableInitialize (
>>> + IN STRING_TABLE *CONST StrTable,
>>> + IN UINTN MaxStringElements
>>> + );
>>> +
>>> +/** Free memory allocated for the string elements in the string table.
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> +
>>> + @return EFI_SUCCESS Success
>>> + @return EFI_INVALID_PARAMETER Invalid string table pointer or
>>> string elements
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +StringTableFree (
>>> + IN STRING_TABLE *CONST StrTable
>>> + );
>>> +
>>> +#endif // SMBIOS_STRING_TABLE_H_
>>> diff --git
>>> a/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
>>> b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
>>>
>>> new file mode 100644
>>> index
>>> 0000000000000000000000000000000000000000..bd186f27fe1613cf819b6600e62960c003a796d6
>>> --- /dev/null
>>> +++
>>> b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.c
>>> @@ -0,0 +1,227 @@
>>> +/** @file
>>> + SMBIOS String Table Helper
>>> +
>>> + Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
>>> +
>>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +
>>> + @par Reference(s):
>>> + - DSP0134 - SMBIOS Specification Version 3.6.0, 2022-06-17
>>> +**/
>>> +
>>> +#include <Library/BaseLib.h>
>>> +#include <Library/BaseMemoryLib.h>
>>> +#include <Library/DebugLib.h>
>>> +#include <Library/MemoryAllocationLib.h>
>>> +#include <Library/SmbiosStringTableLib.h>
>>> +
>>> +/** Add a string to the string table
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> + @param [IN] Str Pointer to the string
>>> + @param [OUT] StrRef Optional pointer to retrieve the string field
>>> + reference of the string in the string table
>>> +
>>> + @return EFI_SUCCESS Success
>>> + @return EFI_INVALID_PARAMETER Invalid string table pointer
>>> + @return EFI_BUFFER_TOO_SMALL Insufficient space to add string
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +StringTableAddString (
>>> + IN STRING_TABLE *CONST StrTable,
>>> + IN CONST CHAR8 *Str,
>>> + OUT UINT8 *StrRef OPTIONAL
>>> + )
>>> +{
>>> + UINTN StrLength;
>>> + STRING_ELEMENT *StrElement;
>>> +
>>> + if ((StrTable == NULL) || (Str == NULL)) {
>>> + return EFI_INVALID_PARAMETER;
>>> + }
>>> +
>>> + if (StrTable->StrCount >= StrTable->MaxStringElements) {
>>> + return EFI_BUFFER_TOO_SMALL;
>>> + }
>>> +
>>> + StrLength = AsciiStrLen (Str);
>>> + if (StrLength == 0) {
>>> + return EFI_INVALID_PARAMETER;
>>> + }
>>> +
>>> + // Update the string element
>>> + StrElement = &StrTable->Elements[StrTable->StrCount];
>>> + StrElement->StringLen = StrLength;
>>> + StrElement->String = Str;
>>> +
>>> + // Update String table information
>>> + StrTable->TotalStrLen += StrLength;
>>> + StrTable->StrCount++;
>>> +
>>> + // Return the index of the string in the string table if requested
>>> + if (StrRef != NULL) {
>>> + // Note: SMBIOS string field references start at 1. So, return the
>>> + // StrCount as the string refrence after it is updated.
>>> + *StrRef = StrTable->StrCount;
>>> + }
>>> +
>>> + return EFI_SUCCESS;
>>> +}
>>> +
>>> +/** Returns the total size required to publish the strings to the
>>> SMBIOS
>>> + string area.
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> +
>>> + @return Total size required to publish the strings in the SMBIOS
>>> string area.
>>> +**/
>>> +UINTN
>>> +EFIAPI
>>> +StringTableGetStringSetSize (
>>> + IN STRING_TABLE *CONST StrTable
>>> + )
>>> +{
>>> + if (StrTable == NULL) {
>>> + ASSERT (0);
>>> + return 0;
>>> + }
>>> +
>>> + // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
>>> + // - If the formatted portion of the structure contains
>>> string-reference
>>> + // fields and all the string fields are set to 0 (no string
>>> references),
>>> + // the formatted section of the structure is followed by two
>>> null (00h)
>>> + // BYTES.
>>> + // - Each string is terminated with a null (00h) BYTE
>>> + // - and the set of strings is terminated with an additional null
>>> (00h) BYTE.
>>> +
>>> + // Therefore, if string count = 0, return 2
>>> + // if string count > 0, the string set size =
>>> + // StrTable->TotalStrLen (total length of the strings in the
>>> string table)
>>> + // + StrTable->StrCount (add string count to include '\0' for each
>>> string)
>>> + // +1 (an additional '\0' is required at the end of the string set).
>>> + return (StrTable->StrCount == 0) ? 2 :
>>> + (StrTable->TotalStrLen + StrTable->StrCount + 1);
>>> +}
>>> +
>>> +/** Iterate through the string table and publish the strings in the
>>> SMBIOS
>>> + string area.
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> + @param [IN] SmbiosStringAreaStart Start address of the SMBIOS
>>> string area.
>>> + @param [IN] SmbiosStringAreaSize Size of the SMBIOS string area.
>>> +
>>> + @return EFI_SUCCESS Success
>>> + @return EFI_INVALID_PARAMETER Invalid string table pointer
>>> + @return EFI_BUFFER_TOO_SMALL Insufficient space to publish strings
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +StringTablePublishStringSet (
>>> + IN STRING_TABLE *CONST StrTable,
>>> + IN CHAR8 *CONST SmbiosStringAreaStart,
>>> + IN CONST UINTN SmbiosStringAreaSize
>>> + )
>>> +{
>>> + UINT8 Index;
>>> + STRING_ELEMENT *StrElement;
>>> + CHAR8 *SmbiosString;
>>> + UINTN BytesRemaining;
>>> + UINTN BytesCopied;
>>> +
>>> + if ((StrTable == NULL) || (SmbiosStringAreaStart == NULL)) {
>>> + return EFI_INVALID_PARAMETER;
>>> + }
>>> +
>>> + if (SmbiosStringAreaSize < StringTableGetStringSetSize (StrTable)) {
>>> + return EFI_BUFFER_TOO_SMALL;
>>> + }
>>> +
>>> + SmbiosString = SmbiosStringAreaStart;
>>> + BytesRemaining = SmbiosStringAreaSize;
> [SAMI] SmbiosString points to the startof the sting storage area.
>>> +
>>> + if (StrTable->StrCount == 0) {
>>> + // See Section 6.1.3 Text strings, SMBIOS Specification Version
>>> 3.6.0
>>> + // If the formatted portion of the structure contains
>>> string-reference
>>> + // fields and all the string fields are set to 0 (no string
>>> references),
>>> + // the formatted section of the structure is followed by two
>>> null (00h)
>>> + // BYTES.
>>> + *SmbiosString++ = '\0';
> [SAMI] Case 1: SmbiosString[0] is set to '\0' and the pointer is
> incremented. So now it points to SmbiosString[1] which will be updated
> with the String table null terminator before the function returns.
>>> + } else {
>>> + for (Index = 0; Index < StrTable->StrCount; Index++) {
>>> + StrElement = &StrTable->Elements[Index];
>>> + AsciiStrCpyS (SmbiosString, BytesRemaining, StrElement->String);
>>> +
>>> + // See Section 6.1.3 Text strings, SMBIOS Specification
>>> Version 3.6.0
>>> + // - Each string is terminated with a null (00h) BYTE
>>> + // Bytes Copied = String length + 1 for the string NULL
>>> terminator.
>>> + BytesCopied = StrElement->StringLen + 1;
>>> + BytesRemaining -= BytesCopied;
>>> + SmbiosString += BytesCopied;
> [SAMI] Case 2: AsciiStrCpyS() copies the string including the
> terrminating null character and BytesCopied is incremented accordingly.
> SmbiosString is then incremented by BytesCopied. So, SmbiosString should
> point to the byte where the next string starts or will be updated with
> the String table null terminator before the function returns.
>>> + }
>>> + }
>>> +
>>> + // See Section 6.1.3 Text strings, SMBIOS Specification Version 3.6.0
>>> + // - the set of strings is terminated with an additional null
>>> (00h) BYTE.
>>> + *SmbiosString = '\0';
>> [GM] Shouldn't you advance the SmbiosString pointer by one more ?
>> After the loop isn't SmbiosString going to be at the NULL char of the
>> last string ? And we're trying to add one more NULL character ?
>> Should it be:
>> SmbiosString++;
> [SAMI] I don't think this increment is required for the reasons
> explained above in Case 1 & Case 2. However, please let me know if I
> have missed something.
>> *SmbiosString = '\0';
>>
[GM]
Got it. Thanks for detail.
Patch looks good to me, I've tested it as well. Thanks a lot.
>>
>>> + return EFI_SUCCESS;
>>> +}
>>> +
>>> +/** Initialise the string table and allocate memory for the string
>>> elements.
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> + @param [IN] MaxStringElements Maximum number of strings that the
>>> string
>>> + table can hold.
>>> +
>>> + @return EFI_SUCCESS Success
>>> + @return EFI_INVALID_PARAMETER Invalid string table pointer
>>> + @return EFI_OUT_OF_RESOURCES Failed to allocate memory for
>>> string elements
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +StringTableInitialize (
>>> + IN STRING_TABLE *CONST StrTable,
>>> + IN UINTN MaxStringElements
>>> + )
>>> +{
>>> + STRING_ELEMENT *Elements;
>>> +
>>> + if ((StrTable == NULL) || (MaxStringElements > MAX_UINT8)) {
>>> + return EFI_INVALID_PARAMETER;
>>> + }
>>> +
>>> + ZeroMem (StrTable, sizeof (STRING_TABLE));
>>> +
>>> + Elements = (STRING_ELEMENT *)AllocateZeroPool (
>>> + sizeof (STRING_ELEMENT) *
>>> MaxStringElements
>>> + );
>>> + if (Elements == NULL) {
>>> + return EFI_OUT_OF_RESOURCES;
>>> + }
>>> +
>>> + StrTable->Elements = Elements;
>>> + StrTable->MaxStringElements = (UINT8)MaxStringElements;
>>> + return EFI_SUCCESS;
>>> +}
>>> +
>>> +/** Free memory allocated for the string elements in the string table.
>>> +
>>> + @param [IN] StrTable Pointer to the string table
>>> +
>>> + @return EFI_SUCCESS Success
>>> + @return EFI_INVALID_PARAMETER Invalid string table pointer or
>>> string elements
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +StringTableFree (
>>> + IN STRING_TABLE *CONST StrTable
>>> + )
>>> +{
>>> + if ((StrTable == NULL) || (StrTable->Elements == NULL)) {
>>> + return EFI_INVALID_PARAMETER;
>>> + }
>>> +
>>> + FreePool (StrTable->Elements);
>>> + ZeroMem (StrTable, sizeof (STRING_TABLE));
>>> + return EFI_SUCCESS;
>>> +}
>>> diff --git
>>> a/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
>>> b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
>>>
>>> new file mode 100644
>>> index
>>> 0000000000000000000000000000000000000000..88624c50e3f3e930074222b6d686f75485538b26
>>> --- /dev/null
>>> +++
>>> b/DynamicTablesPkg/Library/Common/SmbiosStringTableLib/SmbiosStringTableLib.inf
>>> @@ -0,0 +1,25 @@
>>> +## @file
>>> +# SMBIOS String Table Helper library.
>>> +#
>>> +# Copyright (c) 2022, Arm Limited. All rights reserved.
>>> +#
>>> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +##
>>> +
>>> +[Defines]
>>> + INF_VERSION = 0x0001001B
>>> + BASE_NAME = SmbiosStringTableLib
>>> + FILE_GUID = 8C570DD8-531E-473F-85C6-9252746DBAC1
>>> + VERSION_STRING = 1.0
>>> + MODULE_TYPE = DXE_DRIVER
>>> + LIBRARY_CLASS = SmbiosStringTableLib
>>> +
>>> +[Sources]
>>> + SmbiosStringTableLib.c
>>> +
>>> +[Packages]
>>> + MdePkg/MdePkg.dec
>>> + DynamicTablesPkg/DynamicTablesPkg.dec
>>> +
>>> +[LibraryClasses]
>>> + BaseLib
>>> --
>>> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>>>
next prev parent reply other threads:[~2022-10-04 14:38 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-12 14:18 [PATCH v1 1/1] DynamicTablesPkg: Add SMBIOS String table helper library Sami Mujawar
2022-09-12 15:02 ` Sami Mujawar
2022-09-13 3:24 ` [edk2-devel] " Chang, Abner
2022-10-04 3:01 ` Girish Mahadevan
2022-10-04 8:16 ` Sami Mujawar
2022-10-04 14:38 ` Girish Mahadevan [this message]
2022-10-04 8:21 ` PierreGondois
2023-03-09 11:11 ` [edk2-devel] " Sami Mujawar
2023-03-10 7:50 ` PierreGondois
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=cd6246bf-3d24-a5eb-386d-6bc5b5dfd14e@nvidia.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