From: Claudius Heine <claudius.heine.ext@siemens.com>
To: Harald Seiler <hws@denx.de>, isar-users@googlegroups.com
Subject: Re: [PATCH 1/2] Fix all shell-script expansions
Date: Wed, 6 Feb 2019 08:49:39 +0100 [thread overview]
Message-ID: <639d7105-ec7f-d004-ffb3-2514eddb643b@siemens.com> (raw)
In-Reply-To: <20190205130004.15200-2-hws@denx.de>
Hi Harald,
On 05/02/2019 14.00, Harald Seiler wrote:
> Expansions behave weirdly if paths contain special characters.
> This commit fixes expansions in all applicable shell-scripts.
>
> Signed-off-by: Harald Seiler <hws@denx.de>
> ---
> isar-init-build-env | 6 ++--
> meta-isar/conf/distro/raspbian-configscript.sh | 2 +-
> .../isar-bootstrap/files/chroot-setup.sh | 8 ++---
> meta/recipes-devtools/buildchroot/files/common.sh | 2 +-
> meta/recipes-kernel/linux/files/build-kernel.sh | 36 +++++++++++-----------
> .../enable-fsck/files/enable-fsck.sh | 4 +--
> .../files/expand-last-partition.sh | 16 +++++-----
> scripts/ci_build.sh | 14 ++++-----
> scripts/mount_chroot.sh | 12 ++++----
> scripts/rpm2cpio.sh | 4 +--
> 10 files changed, 52 insertions(+), 52 deletions(-)
Great idea!
I think you used shellcheck to find most of those issues. I think you
forgot one:
In ./meta/recipes-kernel/linux-module/files/debian/postinst line 2:
depmod -a $(ls /lib/modules)
^-- SC2046: Quote this to prevent word splitting.
Maybe while you are at it, you could fix this one as well ;)
Thanks,
Claudius
>
> diff --git a/isar-init-build-env b/isar-init-build-env
> index e19d984..b08bb59 100755
> --- a/isar-init-build-env
> +++ b/isar-init-build-env
> @@ -48,10 +48,10 @@ if [ -z "$ISARROOT" ]; then
> fi
> unset THIS_SCRIPT
>
> -ISARROOT=`readlink -f "$ISARROOT"`
> +ISARROOT=$(readlink -f "$ISARROOT")
> export ISARROOT
> -. $ISARROOT/scripts/isar-buildenv-internal $1 && \
> - TEMPLATECONF="$TEMPLATECONF" $ISARROOT/scripts/isar-setup-builddir || {
> +. "$ISARROOT/scripts/isar-buildenv-internal" "$1" && \
> + TEMPLATECONF="$TEMPLATECONF" "$ISARROOT/scripts/isar-setup-builddir" || {
> unset ISARROOT
> return 1
> }
> diff --git a/meta-isar/conf/distro/raspbian-configscript.sh b/meta-isar/conf/distro/raspbian-configscript.sh
> index 3631c26..7a32c5d 100644
> --- a/meta-isar/conf/distro/raspbian-configscript.sh
> +++ b/meta-isar/conf/distro/raspbian-configscript.sh
> @@ -14,7 +14,7 @@ if [ -f /etc/inittab ]; then
> >> /etc/inittab
> fi
>
> -KERNEL_IMAGE=`ls /boot | grep vmlinuz`
> +KERNEL_IMAGE="$(find /boot -maxdepth 1 -name "vmlinuz*" -printf "%p\n")"
>
> cat > /boot/config.txt << EOF
> kernel=$KERNEL_IMAGE
> diff --git a/meta/recipes-core/isar-bootstrap/files/chroot-setup.sh b/meta/recipes-core/isar-bootstrap/files/chroot-setup.sh
> index 9ad86bc..f84f63a 100644
> --- a/meta/recipes-core/isar-bootstrap/files/chroot-setup.sh
> +++ b/meta/recipes-core/isar-bootstrap/files/chroot-setup.sh
> @@ -23,7 +23,7 @@ usage() {
> is prevented.
>
> Usage:
> - $(basename $0) [command] [parameters]
> + $(basename "$0") [command] [parameters]
> commands:
> setup [target path] Setup chroot environment
> cleanup [target path] Cleanup chroot environment
> @@ -132,9 +132,9 @@ main() {
>
> case "${CMD}" in
> "setup")
> - chroot_setup $@;;
> + chroot_setup "$@";;
> "cleanup")
> - chroot_cleanup $@;;
> + chroot_cleanup "$@";;
> *)
> echo "Unknown command '${CMD}'." 1>&2
> echo 1>&2
> @@ -143,4 +143,4 @@ main() {
> esac
> }
>
> -main $@
> +main "$@"
> diff --git a/meta/recipes-devtools/buildchroot/files/common.sh b/meta/recipes-devtools/buildchroot/files/common.sh
> index 25b0bb6..8caa4f4 100644
> --- a/meta/recipes-devtools/buildchroot/files/common.sh
> +++ b/meta/recipes-devtools/buildchroot/files/common.sh
> @@ -24,7 +24,7 @@ else
> fi
>
> # Go to build directory
> -cd $1
> +cd "$1"
>
> # To avoid Perl locale warnings:
> export LC_ALL=C
> diff --git a/meta/recipes-kernel/linux/files/build-kernel.sh b/meta/recipes-kernel/linux/files/build-kernel.sh
> index 745c89a..6e4d347 100644
> --- a/meta/recipes-kernel/linux/files/build-kernel.sh
> +++ b/meta/recipes-kernel/linux/files/build-kernel.sh
> @@ -48,27 +48,27 @@ fi
> rm -f .version
> KBUILD_DEBARCH=$target_arch make -j $(($(nproc) * 2)) deb-pkg
>
> -rm -rf ${REPACK_DIR}
> -mkdir -p ${REPACK_DIR}
> -mkdir -p ${REPACK_LINUX_IMAGE_DIR}
> -mkdir -p ${REPACK_LINUX_HEADERS_DIR}
> +rm -rf "${REPACK_DIR}"
> +mkdir -p "${REPACK_DIR}"
> +mkdir -p "${REPACK_LINUX_IMAGE_DIR}"
> +mkdir -p "${REPACK_LINUX_HEADERS_DIR}"
>
> -cp -a debian ${REPACK_DIR}
> +cp -a debian "${REPACK_DIR}"
>
> # dpkg-gencontrol performs cross-incompatible checks on the
> # Architecture field; trick it to accept the control file
> -sed -i "s/Architecture: .*/Architecture: any/" ${REPACK_DIR}/debian/control
> +sed -i "s/Architecture: .*/Architecture: any/" "${REPACK_DIR}/debian/control"
>
> cd ..
>
> -dpkg-deb -R linux-image-${PV}_${PV}-1_*.deb ${REPACK_LINUX_IMAGE_DIR}
> -dpkg-deb -R linux-headers-${PV}_${PV}-1_*.deb ${REPACK_LINUX_HEADERS_DIR}
> +dpkg-deb -R linux-image-${PV}_${PV}-1_*.deb "${REPACK_LINUX_IMAGE_DIR}"
> +dpkg-deb -R linux-headers-${PV}_${PV}-1_*.deb "${REPACK_LINUX_HEADERS_DIR}"
>
> dpkg-gencontrol -crepack/debian/control \
> -lrepack/debian/changelog \
> -frepack/debian/files \
> -plinux-image-${PV} \
> - -P${REPACK_LINUX_IMAGE_DIR} \
> + -P"${REPACK_LINUX_IMAGE_DIR}" \
> -DPackage="linux-image-${KERNEL_NAME}" \
> -DSection=kernel \
> -DPriority=required \
> @@ -76,8 +76,8 @@ dpkg-gencontrol -crepack/debian/control \
> -DArchitecture=$target_arch
>
> # Add Debian-like link installation to postinst
> -touch ${REPACK_LINUX_IMAGE_DIR}/lib/modules/${PV}/.fresh-install
> -sed -i ${REPACK_LINUX_IMAGE_DIR}/DEBIAN/postinst \
> +touch "${REPACK_LINUX_IMAGE_DIR}/lib/modules/${PV}/.fresh-install"
> +sed -i "${REPACK_LINUX_IMAGE_DIR}/DEBIAN/postinst" \
> -e "/^set -e$/a\\
> \\
> if [ -f /lib/modules/${PV}/.fresh-install ]; then\\
> @@ -89,7 +89,7 @@ linux-update-symlinks \$change ${PV} /boot/vmlinuz-${PV}\\
> rm -f /lib/modules/${PV}/.fresh-install"
>
> # Add Debian-like link removal to postrm
> -sed -i ${REPACK_LINUX_IMAGE_DIR}/DEBIAN/postrm \
> +sed -i "${REPACK_LINUX_IMAGE_DIR}/DEBIAN/postrm" \
> -e "/^set -e$/a\\
> \\
> rm -f /lib/modules/${PV}/.fresh-install\\
> @@ -100,26 +100,26 @@ fi"
>
> # Make sure arm64 kernels are decompressed
> if [ "$target_arch" = "arm64" ]; then
> - vmlinuz=${REPACK_LINUX_IMAGE_DIR}/boot/vmlinuz-${PV}
> - mv $vmlinuz $vmlinuz.gz
> - gunzip $vmlinuz.gz
> + vmlinuz="${REPACK_LINUX_IMAGE_DIR}/boot/vmlinuz-${PV}"
> + mv "$vmlinuz" "$vmlinuz.gz"
> + gunzip "$vmlinuz.gz"
> fi
>
> dpkg-gencontrol -crepack/debian/control \
> -lrepack/debian/changelog \
> -frepack/debian/files \
> -plinux-headers-${PV} \
> - -P${REPACK_LINUX_HEADERS_DIR} \
> + -P"${REPACK_LINUX_HEADERS_DIR}" \
> -Vkernel:debarch="${KERNEL_NAME}" \
> -DPackage="linux-headers-${KERNEL_NAME}" \
> -DSection=kernel \
> -DDepends="${KERNEL_HEADERS_DEBIAN_DEPENDS}" \
> -DArchitecture=$target_arch
>
> -fakeroot dpkg-deb -b ${REPACK_LINUX_IMAGE_DIR} \
> +fakeroot dpkg-deb -b "${REPACK_LINUX_IMAGE_DIR}" \
> linux-image-${KERNEL_NAME}_${PV}-1_${KERNEL_NAME}.deb
> rm -f linux-image-${PV}_${PV}-1_*.deb
> -fakeroot dpkg-deb -b ${REPACK_LINUX_HEADERS_DIR} \
> +fakeroot dpkg-deb -b "${REPACK_LINUX_HEADERS_DIR}" \
> linux-headers-${KERNEL_NAME}_${PV}-1_${KERNEL_NAME}.deb
> rm -f linux-headers-${PV}_${PV}-1_*.deb
>
> diff --git a/meta/recipes-support/enable-fsck/files/enable-fsck.sh b/meta/recipes-support/enable-fsck/files/enable-fsck.sh
> index b3dce44..d09e35d 100644
> --- a/meta/recipes-support/enable-fsck/files/enable-fsck.sh
> +++ b/meta/recipes-support/enable-fsck/files/enable-fsck.sh
> @@ -9,8 +9,8 @@
>
> set -e
>
> -ROOT_DEV=$(/bin/findmnt -n -o SOURCE /)
> -sed -i -e 's|^/dev/root\([ ]\+.*[ ]\+\)0[ ]\+0|'$ROOT_DEV'\10 1|' \
> +ROOT_DEV="$(/bin/findmnt -n -o SOURCE /)"
> +sed -i -e 's|^/dev/root\([ ]\+.*[ ]\+\)0[ ]\+0|'"$ROOT_DEV"'\10 1|' \
> -e 's|^\(/dev/.*[ ]\+\)0[ ]\+0|\10 2|' /etc/fstab
>
> update-initramfs -u
> diff --git a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
> index 913e63b..4cf90de 100644
> --- a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
> +++ b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
> @@ -9,23 +9,23 @@
>
> set -e
>
> -ROOT_DEV=$(findmnt / -o source -n)
> -BOOT_DEV=$(echo ${ROOT_DEV} | sed 's/p\?[0-9]*$//')
> +ROOT_DEV="$(findmnt / -o source -n)"
> +BOOT_DEV="$(echo "${ROOT_DEV}" | sed 's/p\?[0-9]*$//')"
>
> if [ "${ROOT_DEV}" = "${BOOT_DEV}" ]; then
> echo "Boot device equals root device - no partitioning found" >&2
> exit 1
> fi
>
> -LAST_PART=$(sfdisk -d ${BOOT_DEV} 2>/dev/null | tail -1 | cut -d ' ' -f 1)
> +LAST_PART="$(sfdisk -d ${BOOT_DEV} 2>/dev/null | tail -1 | cut -d ' ' -f 1)"
>
> # Remove all hints to the current medium (last-lba) and last partition size,
> # then ask sfdisk to recreate the partitioning
> -sfdisk -d ${BOOT_DEV} 2>/dev/null | grep -v last-lba | \
> - sed 's|\('${LAST_PART}' .*, \)size=[^,]*, |\1|' | \
> - sfdisk --force ${BOOT_DEV}
> +sfdisk -d "${BOOT_DEV}" 2>/dev/null | grep -v last-lba | \
> + sed 's|\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \
> + sfdisk --force "${BOOT_DEV}"
>
> # Inform the kernel about the partitioning change
> -partx -u ${LAST_PART}
> +partx -u "${LAST_PART}"
>
> -resize2fs ${LAST_PART}
> +resize2fs "${LAST_PART}"
> diff --git a/scripts/ci_build.sh b/scripts/ci_build.sh
> index f3523e8..1789183 100755
> --- a/scripts/ci_build.sh
> +++ b/scripts/ci_build.sh
> @@ -13,7 +13,7 @@ ES_BUG=3
> export PATH=$PATH:/sbin
>
> # Go to Isar root
> -cd $(dirname $0)/..
> +cd "$(dirname "$0")/.."
>
> # Start build in Isar tree by default
> BUILD_DIR=./build
> @@ -76,10 +76,10 @@ do
> done
>
> # Setup build folder for the current build
> -if [ ! -d $BUILD_DIR ]; then
> - mkdir -p $BUILD_DIR
> +if [ ! -d "$BUILD_DIR" ]; then
> + mkdir -p "$BUILD_DIR"
> fi
> -source isar-init-build-env $BUILD_DIR
> +source isar-init-build-env "$BUILD_DIR"
>
> if [ -n "$CROSS_BUILD" ]; then
> sed -i -e 's/ISAR_CROSS_COMPILE ?= "0"/ISAR_CROSS_COMPILE ?= "1"/g' conf/local.conf
> @@ -120,9 +120,9 @@ else
> #multiconfig:qemuarm64-buster:isar-image-base
> fi
>
> -cp -a ${ISARROOT}/meta/classes/dpkg-base.bbclass ${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup
> -echo -e "do_fetch_append() {\n\n}" >> ${ISARROOT}/meta/classes/dpkg-base.bbclass
> +cp -a "${ISARROOT}/meta/classes/dpkg-base.bbclass" "${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup"
> +echo -e "do_fetch_append() {\n\n}" >> "${ISARROOT}/meta/classes/dpkg-base.bbclass"
>
> bitbake $BB_ARGS multiconfig:qemuamd64-stretch:isar-image-base
>
> -mv ${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup ${ISARROOT}/meta/classes/dpkg-base.bbclass
> +mv "${ISARROOT}/meta/classes/dpkg-base.bbclass.ci-backup" "${ISARROOT}/meta/classes/dpkg-base.bbclass"
> diff --git a/scripts/mount_chroot.sh b/scripts/mount_chroot.sh
> index 74f2b22..e238f1c 100755
> --- a/scripts/mount_chroot.sh
> +++ b/scripts/mount_chroot.sh
> @@ -2,9 +2,9 @@
>
> set -e
>
> -mount /tmp $1/tmp -o bind
> -mount proc $1/proc -t proc -o nosuid,noexec,nodev
> -mount sysfs $1/sys -t sysfs -o nosuid,noexec,nodev
> -mount devtmpfs $1/dev -t devtmpfs -o mode=0755,nosuid
> -mount devpts $1/dev/pts -t devpts -o gid=5,mode=620
> -mount tmpfs $1/dev/shm -t tmpfs -o rw,seclabel,nosuid,nodev
> +mount /tmp "$1/tmp" -o bind
> +mount proc "$1/proc" -t proc -o nosuid,noexec,nodev
> +mount sysfs "$1/sys" -t sysfs -o nosuid,noexec,nodev
> +mount devtmpfs "$1/dev" -t devtmpfs -o mode=0755,nosuid
> +mount devpts "$1/dev/pts" -t devpts -o gid=5,mode=620
> +mount tmpfs "$1/dev/shm" -t tmpfs -o rw,seclabel,nosuid,nodev
> diff --git a/scripts/rpm2cpio.sh b/scripts/rpm2cpio.sh
> index db3cae6..aa7b7b9 100755
> --- a/scripts/rpm2cpio.sh
> +++ b/scripts/rpm2cpio.sh
> @@ -11,12 +11,12 @@ fatal() {
> }
>
> pkg="$1"
> -[ -n "$pkg" -a -e "$pkg" ] ||
> +[ -n "$pkg" ] && [ -e "$pkg" ] ||
> fatal "No package supplied"
>
> _dd() {
> local o="$1"; shift
> - dd if="$pkg" skip="$o" iflag=skip_bytes status=none $*
> + dd if="$pkg" skip="$o" iflag=skip_bytes status=none "$@"
> }
>
> calcsize() {
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de
next prev parent reply other threads:[~2019-02-06 7:49 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-05 13:00 [PATCH 0/2] Fix shell-scripts Harald Seiler
2019-02-05 13:00 ` [PATCH 1/2] Fix all shell-script expansions Harald Seiler
2019-02-06 7:49 ` Claudius Heine [this message]
2019-02-06 9:27 ` Harald Seiler
2019-02-06 10:08 ` Claudius Heine
2019-02-05 13:00 ` [PATCH 2/2] buildchroot: Fix not checking group Harald Seiler
2019-02-05 18:34 ` Henning Schild
2019-02-06 10:09 ` [PATCH 0/2] Fix shell-scripts Maxim Yu. Osipov
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=639d7105-ec7f-d004-ffb3-2514eddb643b@siemens.com \
--to=claudius.heine.ext@siemens.com \
--cc=hws@denx.de \
--cc=isar-users@googlegroups.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