Lower or Upper Case in EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL (Bugzilla Bug 4866)

[tianocore-issues]

This issue was created automatically with bugzilla2github

Bugzilla Bug 4866

Date: 2024-10-14T11:03:26+00:00
From: Heinrich Schuchardt <<xypron.glpk>>
To: Sam Kaynor <<Sam.Kaynor>>
CC: edhaya.chandran, gaojie, ilias.apalodimas, samer.el-haj-mahmoud, xypron.glpk

Last updated: 2024-12-15T05:19:40+00:00

[tianocore-issues]

Comment 23449

Date: 2024-10-14 11:03:26 +0000
From: Heinrich Schuchardt <<xypron.glpk>>

• Industry Specification: ---
• Release Observed: EDK II Master
• Releases to Fix: EDK II Master
• Target OS: ---
• Bugzilla Assignee(s): Sam Kaynor <<Sam.Kaynor>>

When running the SCT on U-Boot I see errors like:

EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL - ConvertDeviceNodeToText must correctly recover the converting ConvertTextToDeviceNode has acted on the device node string -- PASS
203B6963-5013-4683-958B-D4A21CCCBB8D
/home/user/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c:305: Convert result: MemoryMapped(0xe,0x123456789abcdef,0xfedcba9876543210) - Expected: MemoryMapped(14,0x123456789ABCDEF,0xFEDCBA9876543210)

EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL - ConvertDeviceNodeToText must correctly recover the converting ConvertTextToDeviceNode has acted on the device node string -- PASS
C05C7EBE-69A4-4FCC-B829-257754F3B43E
/home/user/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c:342: Convert result: VenHw(5af6c71e-1261-4637-9838-c4e9913d1dbb,0123456789abcdef) - Expected: VenHw(5AF6C71E-1261-4637-9838-C4E9913D1DBB,0123456789ABCDEF)

EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL - ConvertDeviceNodeToText must correctly recover the converting ConvertTextToDeviceNode has acted on the device node string -- PASS
36DE850B-B28D-4BFD-9EFF-BCD805A4A2F3
/home/user/SctPkg/TestCase/UEFI/EFI/Protocol/DevicePathToText/BlackBoxTest/DevicePathToTextBBTestCoverage.c:377: Convert result: Ctrl(0x1234abcd) - Expected: Ctrl(0x1234ABCD)

Appendix A - GUID AND TIME FORMATS has
"The pairs aa - pp are two characters in the range ‘0’-‘9’, ‘a’-‘f’ or ‘A’-F’, with each pair representing a single byte hexadecimal value."

So the "FAILED" in DevicePathToTextBBTestCoverage.c:342 is definitively a false positive.

I could not see any statement for the EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL requirement that hexadecimal numbers should be upper case. So the other two results seem to be false positives too.

Best regards

Heinrich

[tianocore-issues]

Comment 23516

Date: 2024-11-07 10:26:18 +0000
From: Edhay <<edhaya.chandran>>

Thank you, Heinrich.
The issue is valid.
To be analyzed further and implemented.

Assigned to Sam Kaynor.

[tianocore-issues]

Comment 23553

Date: 2024-12-05 15:28:59 +0000
From: Sam Kaynor <<Sam.Kaynor>>

Hi Heinrich,

After some offline discussion we've come to a couple conclusions about this issue.

The function ConvertDeviceNodeToText comes from the edk2 core and as such can't be easily changed for the purposes of edk2-test. However, the SctCompareMem function used for the test result is passing correctly as it is a strict byte to byte memory comparison (SctPkg/Library/SctLib/Mem.c:133).

As the passes are expected, the issue is in the output presented in the logs. This could be resolved by implementing a local function that simply converts the text output of ConvertDeviceNodeToText to all uppercase characters.

Would this solution address your concerns?

Thanks,

Sam

[tianocore-issues]

Comment 23567

Date: 2024-12-15 05:19:40 +0000
From: Heinrich Schuchardt <<xypron.glpk>>

Hello Sam,

There is nothing wrong in the way the log is printed. The test results for U-Boot are wrong.

The SCT test should be relaxed to accept both lower case and upper case hexadecimal numbers as the UEFI specification does not require the one or the other.

The SCT should convert the actual and the expected result to the same case before calling memcmp().

Best regards

Heinrich

[skaynor]

Hey @xypron ,

I just wanted to clarify the logs you sent at the start of the ticket, where are you seeing a failure? The results you posted are all listed as "PASS" and after some testing on my end using a basic U-Boot shell I can confirm those results are expected and accurate.

The portion of the log that states Convert result: Ctrl(0x1234abcd) - Expected: Ctrl(0x1234ABCD) is correct despite the two results looking to have a case discrepancy. This is because when DevicePathToText->ConvertDeviceNodeToText is called the resulting string is always lowercase, however both DevicePathUtilities->CreateDeviceNode and SctConvertTextToDeviceNode() are case agnostic and create the same hexadecimal value in memory for each DeviceNode, as shown by SctCompareMem when comparing the two nodes in each test.

As such, the SCT test is case agnostic, as the actual and expected results text values don't have to be the same case.

For test clarity, I can understand changing the expected result output in the log to be lowercase, as it would then match up with the output of DevicePathToText->ConvertDeviceNodeToText. Currently it is only uppercase because that portion of the output is hard-coded.

[sunnywang-arm]

We need the failure log from reporter to follow up.

Hi @xypron
Could you share your U-boot test result that shows failure?
It looks like the test case uses SctStrToUInt64() which takes care of case sensitivity. This means that regardless of whether DevicePathToText->ConvertDeviceNodeToText returns uppercase or lowercase characters, the string can still be correctly converted for comparison.

• The code flow to convert the string: SctConvertTextToDeviceNode -> BuildDevPathNodeFunc -> SctStrToUInt64
• The function call to do the comparison: SctCompareMem

Cc @edhay