Re: [ImageBuilder] uboot-script-gen: add xen xsm policy loading support

[Alejandro Vallejo <agarciav@xxxxxxx>]

On Mon Apr 14, 2025 at 12:57 PM BST, Alejandro Vallejo wrote:
> As a general question, why using "test" so much, rather than
>
>    if [ -n "$FOO" -a "FOO" == ABC ]
>
> Using test seems far harder to read than it needs to be, and single
> brackets are perfectly valid POSIX shell.
>
> On Mon Apr 14, 2025 at 9:14 AM BST, Grygorii Strashko wrote:
>> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>
>> This patch adds Xen XSM policy loading support.
>>
>> The configuration file XEN_POLICY specifies Xen hypervisor
>> XSM policy binary to load.
>>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>> ---
>>  README.md                |  2 ++
>>  scripts/uboot-script-gen | 33 +++++++++++++++++++++++++++++++++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/README.md b/README.md
>> index 137abef153ce..9106d2a07302 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -91,6 +91,8 @@ Where:
>>  - XEN specifies the Xen hypervisor binary to load. Note that it has to
>>    be a regular Xen binary, not a u-boot binary.
>>  
>> +- XEN_POLICY specifies the Xen hypervisor XSM policy binary to load.
>> +
>>  - XEN_COLORS specifies the colors (cache coloring) to be used for Xen
>>    and is in the format startcolor-endcolor
>>  
>> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
>> index c4d26caf5e0e..343eba20e4d9 100755
>> --- a/scripts/uboot-script-gen
>> +++ b/scripts/uboot-script-gen
>> @@ -315,6 +315,15 @@ function xen_device_tree_editing()
>>      dt_set "/chosen" "#size-cells" "hex" "0x2"
>>      dt_set "/chosen" "xen,xen-bootargs" "str" "$XEN_CMD"
>>  
>> +    if test "$XEN_POLICY" && test $xen_policy_addr != "-"
>
> If XEN_POLICY is a binary, shouldn't it be "test -f" ? Same later on.

On the absent flag:

Nevermind, I see you're using "-n" implicitly instead. I'd rather it be
less opaque, but it does function. I do think most uses of test "$FOO"
would be better off with an explicit -f or -n. But that's a separate
matter.

Cheers,
Alejandro