Openembedded Core Discussions
 help / color / mirror / Atom feed
From: Jamin Lin <jamin_lin@aspeedtech.com>
To: <openembedded-core@lists.openembedded.org>
Cc: <troy_lee@aspeedtech.com>, <jamin_lin@aspeedtech.com>
Subject: [PATCH v2 2/3] uboot-sign.bbclass: Refactor condition checks to use && and || instead of -a and -o
Date: Tue, 17 Jun 2025 16:10:51 +0800	[thread overview]
Message-ID: <20250617081052.3087995-3-jamin_lin@aspeedtech.com> (raw)
In-Reply-To: <20250617081052.3087995-1-jamin_lin@aspeedtech.com>

This commit cleans up and modernizes the shell condition expressions in
`uboot-sign.bbclass` to follow best practices for portable and reliable shell usage.

Key changes:
- Replace legacy `[ -a ]` and `[ -o ]` with explicit `[ ] && [ ]` and `[ ] || [ ]`.
  Modern POSIX and busybox sh recommend using `&&` and `||` instead of `-a` and `-o`
  because `-a` and `-o` are less robust and can cause parsing ambiguities in some shells.
- Simplify `concat_dtb()` by moving the DTB existence check to the top and using
  early `return` to avoid deep nesting.
- Remove redundant fallback `else` blocks; use clearer control flow with direct checks.

This improves maintainability, reduces shell syntax pitfalls, and aligns with
current shell scripting best practices.

References:
- POSIX recommends avoiding `-a` and `-o` in `[ ]` and using explicit `&&` and `||`:
  https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 meta/classes-recipe/uboot-sign.bbclass | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-recipe/uboot-sign.bbclass
index 01c53c7448..0f387a3a3e 100644
--- a/meta/classes-recipe/uboot-sign.bbclass
+++ b/meta/classes-recipe/uboot-sign.bbclass
@@ -198,21 +198,23 @@ concat_dtb() {
 	# If we're not using a signed u-boot fit, concatenate SPL w/o DTB & U-Boot DTB
 	# with public key (otherwise U-Boot will be packaged by uboot_fitimage_assemble)
 	if [ "${SPL_SIGN_ENABLE}" != "1" ] ; then
-		if [ "x${UBOOT_SUFFIX}" = "ximg" -o "x${UBOOT_SUFFIX}" = "xrom" ] && \
-			[ -e "${UBOOT_DTB_BINARY}" ]; then
+		if [ ! -e "${UBOOT_DTB_BINARY}" ]; then
+			bbwarn "Failure while adding public key to u-boot binary. Verified boot won't be available."
+			return
+		fi
+
+		if [ "x${UBOOT_SUFFIX}" = "ximg" ] || [ "x${UBOOT_SUFFIX}" = "xrom" ]; then
 			oe_runmake EXT_DTB="${UBOOT_DTB_SIGNED}" ${UBOOT_MAKE_TARGET}
 			if [ -n "${binary}" ]; then
 				cp ${binary} ${UBOOT_BINARYNAME}-${type}.${UBOOT_SUFFIX}
 			fi
-		elif [ -e "${UBOOT_NODTB_BINARY}" -a -e "${UBOOT_DTB_BINARY}" ]; then
+		elif [ -e "${UBOOT_NODTB_BINARY}" ]; then
 			if [ -n "${binary}" ]; then
 				cat ${UBOOT_NODTB_BINARY} ${UBOOT_DTB_SIGNED} | tee ${binary} > \
 					${UBOOT_BINARYNAME}-${type}.${UBOOT_SUFFIX}
 			else
 				cat ${UBOOT_NODTB_BINARY} ${UBOOT_DTB_SIGNED} > ${UBOOT_BINARY}
 			fi
-		else
-			bbwarn "Failure while adding public key to u-boot binary. Verified boot won't be available."
 		fi
 	fi
 }
@@ -244,7 +246,7 @@ deploy_dtb() {
 }
 
 concat_spl_dtb() {
-	if [ -e "${SPL_DIR}/${SPL_NODTB_BINARY}" -a -e "${SPL_DIR}/${SPL_DTB_BINARY}" ] ; then
+	if [ -e "${SPL_DIR}/${SPL_NODTB_BINARY}" ] && [ -e "${SPL_DIR}/${SPL_DTB_BINARY}" ] ; then
 		cat ${SPL_DIR}/${SPL_NODTB_BINARY} ${SPL_DIR}/${SPL_DTB_SIGNED} > "${SPL_BINARY}"
 	else
 		bbwarn "Failure while adding public key to spl binary. Verified U-Boot boot won't be available."
@@ -500,7 +502,7 @@ uboot_assemble_fitimage_helper() {
 	type="$1"
 	binary="$2"
 
-	if [ "${UBOOT_SIGN_ENABLE}" = "1" -a -n "${UBOOT_DTB_BINARY}" ] ; then
+	if [ "${UBOOT_SIGN_ENABLE}" = "1" ] && [ -n "${UBOOT_DTB_BINARY}" ] ; then
 		concat_dtb "$type" "$binary"
 	fi
 
@@ -508,7 +510,7 @@ uboot_assemble_fitimage_helper() {
 		uboot_fitimage_assemble
 	fi
 
-	if [ "${SPL_SIGN_ENABLE}" = "1" -a -n "${SPL_DTB_BINARY}" ] ; then
+	if [ "${SPL_SIGN_ENABLE}" = "1" ] && [ -n "${SPL_DTB_BINARY}" ] ; then
 		concat_spl_dtb
 	fi
 }
@@ -547,7 +549,7 @@ addtask uboot_assemble_fitimage before do_install do_deploy after do_compile
 deploy_helper() {
 	type="$1"
 
-	if [ "${UBOOT_SIGN_ENABLE}" = "1" -a -n "${UBOOT_DTB_SIGNED}" ] ; then
+	if [ "${UBOOT_SIGN_ENABLE}" = "1" ] && [ -n "${UBOOT_DTB_SIGNED}" ] ; then
 		deploy_dtb $type
 	fi
 
@@ -569,7 +571,7 @@ deploy_helper() {
 		fi
 	fi
 
-	if [ "${SPL_SIGN_ENABLE}" = "1" -a -n "${SPL_DTB_BINARY}" ] ; then
+	if [ "${SPL_SIGN_ENABLE}" = "1" ] && [ -n "${SPL_DTB_BINARY}" ] ; then
 		deploy_spl_dtb $type
 	fi
 }
@@ -594,7 +596,7 @@ do_deploy:prepend() {
 		deploy_helper ""
 	fi
 
-	if [ "${UBOOT_SIGN_ENABLE}" = "1" -a -n "${UBOOT_DTB_BINARY}" ] ; then
+	if [ "${UBOOT_SIGN_ENABLE}" = "1" ] && [ -n "${UBOOT_DTB_BINARY}" ] ; then
 		ln -sf ${UBOOT_DTB_IMAGE} ${DEPLOYDIR}/${UBOOT_DTB_BINARY}
 		ln -sf ${UBOOT_DTB_IMAGE} ${DEPLOYDIR}/${UBOOT_DTB_SYMLINK}
 		ln -sf ${UBOOT_NODTB_IMAGE} ${DEPLOYDIR}/${UBOOT_NODTB_SYMLINK}
@@ -608,7 +610,7 @@ do_deploy:prepend() {
 		ln -sf ${UBOOT_FITIMAGE_IMAGE} ${DEPLOYDIR}/${UBOOT_FITIMAGE_SYMLINK}
 	fi
 
-	if [ "${SPL_SIGN_ENABLE}" = "1" -a -n "${SPL_DTB_BINARY}" ] ; then
+	if [ "${SPL_SIGN_ENABLE}" = "1" ] && [ -n "${SPL_DTB_BINARY}" ] ; then
 		ln -sf ${SPL_DTB_IMAGE} ${DEPLOYDIR}/${SPL_DTB_SYMLINK}
 		ln -sf ${SPL_DTB_IMAGE} ${DEPLOYDIR}/${SPL_DTB_BINARY}
 		ln -sf ${SPL_NODTB_IMAGE} ${DEPLOYDIR}/${SPL_NODTB_SYMLINK}
-- 
2.43.0



  parent reply	other threads:[~2025-06-17  8:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-17  8:10 [PATCH v2 0/3] Support signing U-Boot FIT image without SPL Jamin Lin
2025-06-17  8:10 ` [PATCH v2 1/3] uboot-sign: " Jamin Lin
2025-06-17  8:10 ` Jamin Lin [this message]
2025-06-17  8:10 ` [PATCH v2 3/3] oe-selftest: fitimage: Add test for " Jamin Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250617081052.3087995-3-jamin_lin@aspeedtech.com \
    --to=jamin_lin@aspeedtech.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=troy_lee@aspeedtech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox