All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.