[OE-core] [PATCH] kernel: fitimage: Repair misuse of shell test command

[Andreas Oberritter]

Hi Marek,

On 08.05.2016 13:21, Marek Vasut wrote:
> The kernel fitImage must be amended with signature if and only if
> UBOOT_SIGN_ENABLE = 1 . In the current case, the UBOOT_SIGN_ENABLE
> could be either 0 (default) or 1 , which test -n always correctly
> interprets as non-empty string, thus always true. This does not
> match the logic above though, so replace the test with check which
> passes only for UBOOT_SIGN_ENABLE = 1 .
> 
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Yannick Gicquel <yannick.gicquel at iot.bzh>
> Cc: Richard Purdie <richard.purdie at linuxfoundation.org>
> ---
>  meta/classes/kernel-fitimage.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> ---
> NOTE: It'd be real nice if I was CCed on the original patches
> NOTE: I'm not convinced that UBOOT_SIGN_ENABLE is the right name
>       for this variable, since the signed object is really the
>       fitImage and U-Boot only verifies the signature. Maybe we
>       should rename it, is it still possible ?
> 
> diff --git a/meta/classes/kernel-fitimage.bbclass b/meta/classes/kernel-fitimage.bbclass
> index 809bd4d..298eda2 100644
> --- a/meta/classes/kernel-fitimage.bbclass
> +++ b/meta/classes/kernel-fitimage.bbclass
> @@ -250,7 +250,7 @@ do_assemble_fitimage() {
>  		#
>  		# Step 5: Sign the image and add public key to U-Boot dtb
>  		#
> -		if test -n "${UBOOT_SIGN_ENABLE}"; then
> +		if [ "x${UBOOT_SIGN_ENABLE}" = "x1" ] ; then

just a nitpick about coding style, but I think you should remove the
extra x. Most people add it, because they have seen it somewhere else,
but it doesn't make much sense if the variable is both enclosed in
quotes and has very little likeliness to get set to a valid option for
'test' returning true.

Leaving the extra x out improves readability for shell novices and, in
general (but not in this case), the ability to grep using whole word
matches.

Regards,
Andreas