From c7d1704feb918875d74ba129e3f9eaf6ea469574 Mon Sep 17 00:00:00 2001
From: Daniel Golle <daniel@makrotopia.org>
Date: Sat, 15 May 2021 23:30:23 +0100
Subject: [PATCH] uvol: try to be more shellcheck compliant, fix LVM vol-up

 * quotes around all variables
 * always use 'read -r' instead of 'read'
 * some more minor shellcheck fixes
 * reorder LVM ops for 'up' call to allow it to succeed

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 utils/uvol/files/autopart.defaults |  30 +++----
 utils/uvol/files/lvm.sh            |  18 ++--
 utils/uvol/files/ubi.sh            | 138 +++++++++++++++--------------
 utils/uvol/files/uvol              |   2 +-
 4 files changed, 97 insertions(+), 91 deletions(-)

diff --git a/utils/uvol/files/autopart.defaults b/utils/uvol/files/autopart.defaults
index b6a3cdd603..0df2829c67 100644
--- a/utils/uvol/files/autopart.defaults
+++ b/utils/uvol/files/autopart.defaults
@@ -26,12 +26,12 @@ get_partition_by_name_gpt() {
 }
 
 part_fixup() {
-	echo "write" | sfdisk --force -q -w never $1
+	echo "write" | sfdisk --force -q -w never "$1"
 }
 
 get_free_area() {
 	local found=
-	sfdisk -q -F "$1" 2>/dev/null | while read start end sectors size; do
+	sfdisk -q -F "$1" 2>/dev/null | while read -r start end sectors size; do
 		case $start in
 		*"Unpartitioned"* | *"Units:"* | *"Sector"* | *"Start"* )
 			continue
@@ -55,13 +55,13 @@ get_free_area() {
 }
 
 create_lvm_part() {
-	local disk=$1
+	local disk="$1"
 	local freepart
 
-	freepart="$(get_free_area $disk)"
+	freepart="$(get_free_area "$disk")"
 	if [ "$freepart" ]; then
-		echo "$freepart, type=lvm, name=$OWRT_VOLUMES" | sfdisk --force -w never -a $disk
-		partx -a $disk 1>/dev/null 2>/dev/null || true
+		echo "$freepart, type=lvm, name=$OWRT_VOLUMES" | sfdisk --force -w never -a "$disk"
+		partx -a "$disk" 1>/dev/null 2>/dev/null || true
 		return 0
 	else
 		return 1
@@ -69,8 +69,8 @@ create_lvm_part() {
 }
 
 lvm_init() {
-	lvm pvcreate -f $1
-	lvm vgcreate "$2" $1
+	lvm pvcreate -f "$1"
+	lvm vgcreate "$2" "$1"
 	lvm vgs
 }
 
@@ -83,16 +83,16 @@ autopart_init() {
 
 	[ "$diskdev" ] || return
 
-	[ -e "/sys/class/block/$diskdev/device/serial" ] && diskserial="$(cat /sys/class/block/$diskdev/device/serial)"
-	[ -e "/sys/class/block/$diskdev/device/cid" ] && diskserial="$diskserial$(cat /sys/class/block/$diskdev/device/cid)"
+	[ -e "/sys/class/block/$diskdev/device/serial" ] && diskserial="$(cat "/sys/class/block/$diskdev/device/serial")"
+	[ -e "/sys/class/block/$diskdev/device/cid" ] && diskserial="$diskserial$(cat "/sys/class/block/$diskdev/device/cid")"
 	[ "$diskserial" ] || diskserial="$(cat /proc/sys/kernel/random/uuid)"
-	diskhash="$(echo $diskserial | sha256sum | cut -d' ' -f1)"
-	part_fixup /dev/$diskdev
-	create_lvm_part /dev/$diskdev || return
-	lvmpart=$(get_partition_by_name_gpt $diskdev $OWRT_VOLUMES)
+	diskhash="$(echo "$diskserial" | sha256sum | cut -d' ' -f1)"
+	part_fixup "/dev/$diskdev"
+	create_lvm_part "/dev/$diskdev" || return
+	lvmpart="$(get_partition_by_name_gpt "$diskdev" "$OWRT_VOLUMES")"
 
 	[ "$lvmpart" ] || return
-	lvm_init $lvmpart "${OWRT_VOLUMES}-${diskhash:0:16}"
+	lvm_init "$lvmpart" "${OWRT_VOLUMES}-${diskhash:0:16}"
 }
 
 autopart_init
diff --git a/utils/uvol/files/lvm.sh b/utils/uvol/files/lvm.sh
index c250be534b..4b295faf17 100644
--- a/utils/uvol/files/lvm.sh
+++ b/utils/uvol/files/lvm.sh
@@ -46,11 +46,11 @@ lvs() {
 }
 
 freebytes() {
-	echo $(($vg_free_count * $vg_extent_size * 1024))
+	echo $((vg_free_count * vg_extent_size * 1024))
 }
 
 totalbytes() {
-	echo $(($vg_extent_count * $vg_extent_size * 1024))
+	echo $((vg_extent_count * vg_extent_size * 1024))
 }
 
 existvol() {
@@ -148,12 +148,12 @@ exportlv() {
 getdev() {
 	existvol "$1" || return 1
 	exportlv "$1"
-	echo $lv_dm_path
+	echo "$lv_dm_path"
 }
 
 getsize() {
 	exportlv "$1"
-	[ "$lv_size" ] && echo $lv_size
+	[ "$lv_size" ] && echo "$lv_size"
 }
 
 activatevol() {
@@ -166,8 +166,8 @@ activatevol() {
 			;;
 		*)
 			[ "$lv_active" = "active" ] && return 0
-			lvm_cmd lvchange -a y "$lv_full_name" || return $?
 			lvm_cmd lvchange -k n "$lv_full_name" || return $?
+			lvm_cmd lvchange -a y "$lv_full_name" || return $?
 			ubus send block.volume "{\"name\": \"$1\", \"action\": \"up\", \"mode\": \"${lv_name:0:2}\", \"device\": \"$lv_dm_path\"}"
 			return 0
 			;;
@@ -222,7 +222,7 @@ createvol() {
 			;;
 	esac
 
-	lvm_cmd lvcreate -p $lvmode -a n -y -W n -Z n -n "${mode}_$1" -l "$size_ext" $vg_name
+	lvm_cmd lvcreate -p "$lvmode" -a n -y -W n -Z n -n "${mode}_$1" -l "$size_ext" "$vg_name"
 	ret=$?
 	if [ ! $ret -eq 0 ] || [ "$lvmode" = "r" ]; then
 		return $ret
@@ -230,7 +230,7 @@ createvol() {
 	exportlv "$1"
 	[ "$lv_full_name" ] || return 22
 	lvm_cmd lvchange -a y "$lv_full_name" || return 1
-	if [ $lv_size -gt $(( 100 * 1024 * 1024 )) ]; then
+	if [ "$lv_size" -gt $(( 100 * 1024 * 1024 )) ]; then
 		mkfs.f2fs -f -l "$1" "$lv_path"
 		ret=$?
 		[ $ret != 0 ] && [ $ret != 134 ] && return 1
@@ -253,11 +253,11 @@ removevol() {
 updatevol() {
 	exportlv "$1"
 	[ "$lv_full_name" ] || return 2
-	[ $lv_size -ge $2 ] || return 27
+	[ "$lv_size" -ge "$2" ] || return 27
 	case "$lv_path" in
 		/dev/*/wo_*)
 			lvm_cmd lvchange -a y -p rw "$lv_full_name"
-			dd of=$lv_path
+			dd of="$lv_path"
 			lvm_cmd lvchange -p r "$lv_full_name"
 			lvm_cmd lvrename "$lv_full_name" "${lv_full_name%%/*}/ro_$1"
 			ubus send block.volume "{\"name\": \"$1\", \"action\": \"up\", \"mode\": \"ro\", \"device\": \"$(getdev "$@")\"}"
diff --git a/utils/uvol/files/ubi.sh b/utils/uvol/files/ubi.sh
index 2da7e309b8..0664ce9350 100644
--- a/utils/uvol/files/ubi.sh
+++ b/utils/uvol/files/ubi.sh
@@ -9,28 +9,28 @@ if [ "$cmd" = "name" ]; then
 fi
 
 test -e /sys/class/ubi/version || return 0
-read ubiver < /sys/class/ubi/version
+read -r ubiver < /sys/class/ubi/version
 [ "$ubiver" = "1" ] || return 1
 test -e /sys/devices/virtual/ubi || return 0
 
 ubidev=$(ls -1 /sys/devices/virtual/ubi | head -n 1)
 
-read ebsize < "/sys/devices/virtual/ubi/${ubidev}/eraseblock_size"
+read -r ebsize < "/sys/devices/virtual/ubi/${ubidev}/eraseblock_size"
 
 freebytes() {
-	read availeb < "/sys/devices/virtual/ubi/${ubidev}/avail_eraseblocks"
+	read -r availeb < "/sys/devices/virtual/ubi/${ubidev}/avail_eraseblocks"
 	echo $((availeb * ebsize))
 }
 
 totalbytes() {
-	read totaleb < "/sys/devices/virtual/ubi/${ubidev}/total_eraseblocks"
+	read -r totaleb < "/sys/devices/virtual/ubi/${ubidev}/total_eraseblocks"
 	echo $((totaleb * ebsize))
 }
 
 getdev() {
-	local voldir volname devname
-	for voldir in /sys/devices/virtual/ubi/${ubidev}/${ubidev}_*; do
-		read volname < "${voldir}/name"
+	local voldir volname
+	for voldir in "/sys/devices/virtual/ubi/${ubidev}/${ubidev}_"*; do
+		read -r volname < "${voldir}/name"
 		case "$volname" in
 			uvol-[rw][owpd]-$1)
 				basename "$voldir"
@@ -46,7 +46,7 @@ getdev() {
 vol_is_mode() {
 	local voldev="$1"
 	local volname
-	read volname < "/sys/devices/virtual/ubi/${ubidev}/${voldev}/name"
+	read -r volname < "/sys/devices/virtual/ubi/${ubidev}/${voldev}/name"
 	case "$volname" in
 		uvol-$2-*)
 			return 0
@@ -56,42 +56,45 @@ vol_is_mode() {
 }
 
 getstatus() {
-	local voldev=$(getdev "$@")
+	local voldev
+	voldev="$(getdev "$@")"
 	[ "$voldev" ] || return 2
-	vol_is_mode $voldev wo && return 22
-	vol_is_mode $voldev wp && return 16
-	vol_is_mode $voldev wd && return 1
-	vol_is_mode $voldev ro && [ ! -e "/dev/ubiblock${voldev:3}" ] && return 1
+	vol_is_mode "$voldev" wo && return 22
+	vol_is_mode "$voldev" wp && return 16
+	vol_is_mode "$voldev" wd && return 1
+	vol_is_mode "$voldev" ro && [ ! -e "/dev/ubiblock${voldev:3}" ] && return 1
 	return 0
 }
 
 getsize() {
 	local voldev
-	voldev=$(getdev "$@")
+	voldev="$(getdev "$@")"
 	[ "$voldev" ] || return 2
-	cat /sys/devices/virtual/ubi/${ubidev}/${voldev}/data_bytes
+	cat "/sys/devices/virtual/ubi/${ubidev}/${voldev}/data_bytes"
 }
 
 getuserdev() {
-	local voldev=$(getdev "$@")
+	local voldev
+	voldev="$(getdev "$@")"
 	[ "$voldev" ] || return 2
-	if vol_is_mode $voldev ro ; then
+	if vol_is_mode "$voldev" ro ; then
 		echo "/dev/ubiblock${voldev:3}"
-	elif vol_is_mode $voldev rw ; then
+	elif vol_is_mode "$voldev" rw ; then
 		echo "/dev/$voldev"
 	fi
 }
 
 mkubifs() {
-	local tmp_mp=$(mktemp -d)
-	mount -t ubifs $1 $tmp_mp
-	umount $tmp_mp
-	rmdir $tmp_mp
+	local tmp_mp
+	tmp_mp="$(mktemp -d)"
+	mount -t ubifs "$1" "$tmp_mp"
+	umount "$tmp_mp"
+	rmdir "$tmp_mp"
 }
 
 createvol() {
-	local mode ret
-	local voldev=$(getdev "$@")
+	local mode ret voldev
+	voldev=$(getdev "$@")
 	[ "$voldev" ] && return 17
 	case "$3" in
 		ro|wo)
@@ -104,105 +107,108 @@ createvol() {
 			return 22
 			;;
 	esac
-	ubimkvol /dev/$ubidev -N "uvol-$mode-$1" -s "$2"
+	ubimkvol "/dev/$ubidev" -N "uvol-$mode-$1" -s "$2"
 	ret=$?
 	[ $ret -eq 0 ] || return $ret
-	voldev=$(getdev "$@")
-	ubiupdatevol -t /dev/$voldev
+	voldev="$(getdev "$@")"
+	ubiupdatevol -t "/dev/$voldev"
 	[ "$mode" = "wp" ] || return 0
-	mkubifs /dev/$voldev
-	ubirename /dev/$ubidev uvol-wp-$1 uvol-rw-$1
+	mkubifs "/dev/$voldev"
+	ubirename "/dev/$ubidev" "uvol-wp-$1" "uvol-rw-$1"
 	ubus send block.volume "{\"name\": \"$1\", \"action\": \"up\", \"mode\": \"rw\", \"fstype\": \"ubifs\", \"device\": \"/dev/$voldev\"}"
 }
 
 removevol() {
-	local voldev=$(getdev "$@")
-	local evdata
+	local voldev evdata
+	voldev=$(getdev "$@")
 	[ "$voldev" ] || return 2
-	if vol_is_mode $voldev rw ; then
+	if vol_is_mode "$voldev" rw ; then
 		evdata="{\"name\": \"$1\", \"action\": \"down\", \"device\": \"/dev/$voldev\"}"
-	elif vol_is_mode $voldev ro && [ -e "/dev/ubiblock${voldev:3}" ]; then
+	elif vol_is_mode "$voldev" ro && [ -e "/dev/ubiblock${voldev:3}" ]; then
 		evdata="{\"name\": \"$1\", \"action\": \"down\", \"device\": \"/dev/ubiblock${voldev:3}\"}"
 	fi
-	local volnum=${voldev#${ubidev}_}
-	ubirmvol /dev/$ubidev -n $volnum || return $?
+	local volnum="${voldev#${ubidev}_}"
+	ubirmvol "/dev/$ubidev" -n "$volnum" || return $?
 	[ "$evdata" ] && ubus send block.volume "$evdata"
 }
 
 activatevol() {
-	local voldev=$(getdev "$@")
+	local voldev
+	voldev="$(getdev "$@")"
 	[ "$voldev" ] || return 2
-	vol_is_mode $voldev rw && return 0
-	vol_is_mode $voldev wo && return 22
-	vol_is_mode $voldev wp && return 16
-	if vol_is_mode $voldev ro; then
+	vol_is_mode "$voldev" rw && return 0
+	vol_is_mode "$voldev" wo && return 22
+	vol_is_mode "$voldev" wp && return 16
+	if vol_is_mode "$voldev" ro; then
 		[ -e "/dev/ubiblock${voldev:3}" ] && return 0
-		ubiblock --create /dev/$voldev
+		ubiblock --create "/dev/$voldev"
 		ubus send block.volume "{\"name\": \"$1\", \"action\": \"up\", \"mode\": \"ro\", \"device\": \"/dev/ubiblock${voldev:3}\"}"
 		return 0
-	elif vol_is_mode $voldev wd; then
-		ubirename /dev/$ubidev uvol-wd-$1 uvol-rw-$1
+	elif vol_is_mode "$voldev" wd; then
+		ubirename "/dev/$ubidev" "uvol-wd-$1" "uvol-rw-$1"
 		ubus send block.volume "{\"name\": \"$1\", \"action\": \"up\", \"mode\": \"rw\", \"fstype\": \"ubifs\", \"device\": \"/dev/$voldev\"}"
 		return 0
 	fi
 }
 
 disactivatevol() {
-	local voldev=$(getdev "$@")
+	local voldev
+	voldev="$(getdev "$@")"
 	[ "$voldev" ] || return 2
-	vol_is_mode $voldev wo && return 22
-	vol_is_mode $voldev wp && return 16
-	if vol_is_mode $voldev ro; then
+	vol_is_mode "$voldev" wo && return 22
+	vol_is_mode "$voldev" wp && return 16
+	if vol_is_mode "$voldev" ro; then
 		[ -e "/dev/ubiblock${voldev:3}" ] || return 0
-		ubiblock --remove /dev/$voldev || return $?
+		ubiblock --remove "/dev/$voldev" || return $?
 		ubus send block.volume "{\"name\": \"$1\", \"action\": \"down\", \"mode\": \"ro\", \"device\": \"/dev/ubiblock${voldev:3}\"}"
 		return 0
-	elif vol_is_mode $voldev rw; then
-		ubirename /dev/$ubidev uvol-rw-$1 uvol-wd-$1 || return $?
+	elif vol_is_mode "$voldev" rw; then
+		ubirename "/dev/$ubidev" "uvol-rw-$1" "uvol-wd-$1" || return $?
 		ubus send block.volume "{\"name\": \"$1\", \"action\": \"down\", \"mode\": \"rw\", \"device\": \"/dev/$voldev\"}"
 		return 0
 	fi
 }
 
 updatevol() {
-	local voldev=$(getdev "$@")
+	local voldev
+	voldev="$(getdev "$@")"
 	[ "$voldev" ] || return 2
 	[ "$2" ] || return 22
-	vol_is_mode $voldev wo || return 22
-	ubiupdatevol -s $2 /dev/$voldev -
-	ubirename /dev/$ubidev uvol-wo-$1 uvol-ro-$1
-	ubiblock --create /dev/$voldev
+	vol_is_mode "$voldev" wo || return 22
+	ubiupdatevol -s "$2" "/dev/$voldev" -
+	ubirename "/dev/$ubidev" "uvol-wo-$1" "uvol-ro-$1"
+	ubiblock --create "/dev/$voldev"
 	ubus send block.volume "{\"name\": \"$1\", \"action\": \"up\", \"mode\": \"ro\", \"device\": \"/dev/ubiblock${voldev:3}\"}"
 }
 
 listvols() {
 	local volname volmode volsize
-	for voldir in /sys/devices/virtual/ubi/${ubidev}/${ubidev}_*; do
-		read volname < $voldir/name
+	for voldir in "/sys/devices/virtual/ubi/${ubidev}/${ubidev}_"*; do
+		read -r volname < "$voldir/name"
 		case "$volname" in
 			uvol-[rw][wod]*)
-				read volsize < $voldir/data_bytes
+				read -r volsize < "$voldir/data_bytes"
 				;;
 			*)
 				continue
 				;;
 		esac
-		volmode=${volname:5:2}
-		volname=${volname:8}
+		volmode="${volname:5:2}"
+		volname="${volname:8}"
 		echo "$volname $volmode $volsize"
 	done
 }
 
 bootvols() {
 	local volname volmode volsize voldev fstype
-	for voldir in /sys/devices/virtual/ubi/${ubidev}/${ubidev}_*; do
-		read volname < $voldir/name
-		voldev=$(basename $voldir)
+	for voldir in "/sys/devices/virtual/ubi/${ubidev}/${ubidev}_"*; do
+		read -r volname < "$voldir/name"
+		voldev="$(basename "$voldir")"
 		fstype=
 		case "$volname" in
 			uvol-ro-*)
 				voldev="/dev/ubiblock${voldev:3}"
-				ubiblock --create /dev/$voldev
+				ubiblock --create "/dev/$voldev"
 				;;
 			uvol-rw-*)
 				voldev="/dev/$voldev"
@@ -212,8 +218,8 @@ bootvols() {
 				continue
 				;;
 		esac
-		volmode=${volname:5:2}
-		volname=${volname:8}
+		volmode="${volname:5:2}"
+		volname="${volname:8}"
 		ubus send block.volume "{\"name\": \"$volname\", \"action\": \"up\", \"mode\": \"$volmode\",${fstype:+ \"fstype\": \"$fstype\", }\"device\": \"$voldev\"}"
 	done
 }
diff --git a/utils/uvol/files/uvol b/utils/uvol/files/uvol
index 4958d31ff4..04547ce622 100644
--- a/utils/uvol/files/uvol
+++ b/utils/uvol/files/uvol
@@ -49,4 +49,4 @@ if [ -z "$uvol_backend" ]; then
 	return 2
 fi
 
-flock -x /tmp/run/uvol.lock $uvol_backend "$@"
+flock -x /tmp/run/uvol.lock "$uvol_backend" "$@"