Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add new JedecJep106Lib to fetch JEDEC JEP106 manufacturer

["Rebecca Cran" <rebecca@bsdio.com>]

I guess GLOBAL_REMOVE_IF_UNREFERENCED should be added to the struct too?


On 4/7/23 4:42 PM, Rebecca Cran wrote:
> On 4/7/23 2:25 PM, Kinney, Michael D wrote:
>> Comments below.
>>
>> Hopefully this lib would only be used by modules that get compressed.
> I guess so, but that's for the user to decide.
>>
>> Might add GLOBAL_REMOVE_IF_UNREFERENCED to the arrays of strings to
>> help the optimizer remove the data that is not referenced.
>
> Good idea - I'll add that.
>
>>> +CONST CHAR8 *
>>> +EFIAPI
>>> +Jep106GetManufacturerName (
>>> +  IN UINT8  Code,
>>> +  IN UINT8  ContinuationBytes
>>> +  )
>>> +{
>>> +  UINTN                      Index;
>>> +  CONST JEDEC_MANUFACTURERS  *ManufacturersBank;
>>> +
>>> +  Index = 0;
>>> +
>>> +  if (ContinuationBytes >= JEP106_MANUFACTURERS_NUM_BANKS) {
>>> +    ASSERT (0);
>> Do you really want an ASSERT() from this?  If this is data from a DIMM
>> I doubt we want platform to ASSERT().  Perhaps just return NULL?
> I'll remove it. I added it for validation when I was writing the SPD 
> parsing library.
>>> +UINTN
>>> +EFIAPI
>>> +Jep106GetLongestManufacturerName (
>>> +  VOID
>>> +  )
>> Why is this API needed?  Wouldn’t you really just need the
>> longest of the ones present in the current boot to build the
>> SMBIOS record?.  Not the longest of all in the banks?
>
> I added it because it's useful to know the maximum possible size of 
> the strings that could be in the SMBIOS Type 17 table before doing the 
> SPD parsing.
>
> I'll remove it for now, and if it's really needed I can add it back in.
>
>