[PATCH v1 1/2] test: fix print_guid()

[Jerome Forissier]

Hi Heinrich,

On 4/15/25 15:25, Heinrich Schuchardt wrote:
> On 15.04.25 14:53, Jerome Forissier wrote:
>> The %pUs format specifier prints the system partition GUID as "System
>> Partition" if PARTITION_TYPE_GUID is disabled but either CMD_EFIDEBUG
>> or EFI are enabled. See list_guid[] in lib/uuid.c. Fix the print_guid()
>> unit test accordingly.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier at linaro.org>
>> ---
>>
>>   test/common/print.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/test/common/print.c b/test/common/print.c
>> index e3711b10809..574c84b2eee 100644
>> --- a/test/common/print.c
>> +++ b/test/common/print.c
>> @@ -47,6 +47,8 @@ static int print_guid(struct unit_test_state *uts)
>>       sprintf(str, "%pUs", guid_esp);
>>       if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID)) { /* brace needed */
> 
> I would not know why a brace should be needed for a define using
> {( some expressions }).
> 
> The problem should be resolved since fa847bb409d6 ("test: Wrap assert macros in ({ ... }) and fix missing semicolons").

Most probably, yes. I'll reformat in v2.

>>           ut_asserteq_str("system", str);
>> +    } else if (IS_ENABLED(CONFIG_CMD_EFIDEBUG) || IS_ENABLED(CONFIG_EFI)) {
>> +        ut_asserteq_str("System Partition", str);
> 
> Printing two different strings depending on configuration settings is not very helpful for users.

Agreed.

> Can we print "ESP" in all cases, please?

Fine with me in principle but...
- Don't we risk breaking things which would depend on the partition name?
- I find "ESP" a bit cryptic. I would suggest "EFI System Partition" but it
is quite long and I see that CONFIG_PARTITION_TYPE_GUID defines only short
names ("system", "mbr", "msft"...) so there must be a reason.

Regards,
-- 
Jerome


> Best regards
> 
> Heinrich
> 
>>       } else {
>>           ut_asserteq_str("c12a7328-f81f-11d2-ba4b-00a0c93ec93b", str);
>>       }
>