Fix shellcheck errors in chromeos-touch-common.sh
Updated code to match recommended shell code best practices.
BUG=b:259595539
TEST=CQ
TEST=Deployed on Starmie, Zydron and used to downgrade+upgrade
touch firmware.
Change-Id: I22eedc67fe1919f3da31db76f1771bf07a0affed
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/touch_updater/+/5018303
Commit-Queue: Henry Barnor <hbarnor@chromium.org>
Reviewed-by: Kenneth Albanowski <kenalba@google.com>
Tested-by: Henry Barnor <hbarnor@chromium.org>
diff --git a/common/scripts/chromeos-touch-common.sh b/common/scripts/chromeos-touch-common.sh
index 8d6e3a3..76a2242 100755
--- a/common/scripts/chromeos-touch-common.sh
+++ b/common/scripts/chromeos-touch-common.sh
@@ -4,6 +4,9 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
+# Touch firmware vendor ids.
+PIXART_VENDOR_ID="093A"
+
I2C_DEVICES_PATH="/sys/bus/i2c/devices"
# powerd will check this lock file and block device suspension.
@@ -46,12 +49,13 @@
local required_sysfs="$2"
for dev in "${I2C_DEVICES_PATH}"/*/name; do
- local dev_name="$(cat "${dev}")"
+ local dev_name
+ dev_name="$(cat "${dev}")"
if [ "${name_to_find}" = "${dev_name}" ]; then
local missing_sysfs_entry=0
local path="${dev%/name}"
- for sysfs in $required_sysfs; do
+ for sysfs in ${required_sysfs}; do
if [ ! -e "${path}/${sysfs}" ]; then
missing_sysfs_entry=1
fi
@@ -74,12 +78,15 @@
local preferred_search_path="${2}"
for dev in ${preferred_search_path} "${I2C_DEVICES_PATH}"/*/; do
- local driver_name="$(readlink -f ${dev}/driver | xargs basename)"
+ local driver_name
+ driver_name="$(readlink -f "${dev}"/driver | xargs basename)"
case "${driver_name}" in
i2c_hid*)
- local hid_path=$(echo ${dev}/*:${vid}:${pid}.*)
+ local hid_path
+ hid_path=$(echo "${dev}"/*:"${vid}":"${pid}".*)
if [ -d "${hid_path}" ]; then
- local hidraw_sysfs_path=$(echo ${hid_path}/hidraw/hidraw*)
+ local hidraw_sysfs_path
+ hidraw_sysfs_path=$(echo "${hid_path}"/hidraw/hidraw*)
path="/dev/${hidraw_sysfs_path##*/}"
break
fi
@@ -110,8 +117,9 @@
local specific_fw_link_path=""
local specific_fw_link_path2=""
local generic_fw_link_path=""
- local fw_link_name_extension="`expr "$fw_link_name" : ".*\(\..*\)"`"
- local fw_link_name_base="${fw_link_name%$fw_link_name_extension}"
+ local fw_link_name_extension
+ fw_link_name_extension="$(expr "${fw_link_name}" : ".*\(\..*\)")"
+ local fw_link_name_base="${fw_link_name%"${fw_link_name_extension}"}"
case ${fw_link_name_base} in
/*) fw_link_path="${fw_link_name_base}" ;;
@@ -143,7 +151,7 @@
for i in $(seq 5); do
printf 1 > "${touch_device_path}/update_fw"
ret=$?
- if [ ${ret} -eq 0 ]; then
+ if [ "${ret}" -eq 0 ]; then
return 0
fi
log_msg "update_firmware try #${i} failed... retrying."
@@ -184,7 +192,7 @@
driver_path="$1"; [ "$#" -eq 0 ] || shift
# Fallback for updaters which do not specify the driver_path arg.
- if [ -z "$driver_path" ]; then
+ if [ -z "${driver_path}" ]; then
driver_path="$(readlink -f "${touch_device_path}/driver")"
fi
@@ -193,15 +201,15 @@
log_msg "Attempting to re-bind '${bus_id}' to driver '${driver_path}'"
echo "${bus_id}" > "${driver_path}/unbind"
ret="$?"
- if [ "$ret" -ne 0 ]; then
- log_msg "Unable to unbind the device from the driver, error $ret. This "\
-"is sometimes expected when recovering from corrupted firmware. Continuing "\
-"with attempt to bind the device to the driver."
+ if [ "${ret}" -ne 0 ]; then
+ log_msg "Unable to unbind the device from the driver, error ${ret}. This " \
+ "is sometimes expected when recovering from corrupted firmware. " \
+ "Continuing with attempt to bind the device to the driver."
fi
echo "${bus_id}" > "${driver_path}/bind"
ret="$?"
- if [ "$ret" -ne 0 ]; then
+ if [ "${ret}" -ne 0 ]; then
log_msg "Unable to bind the device to the driver."
return 1
fi
@@ -213,8 +221,16 @@
printf "%d" "0x""$1"
}
+# Disable SC2145 due to the fact that there is no way to fix it without
+# side-effects. We would be guessing at the intent of the authors.
+# SC2154 is also disabled so that all the fixes for this function
+# can be done together.
+#
+# TODO: b/314386667 - Remove shellcheck disable.
+# shellcheck disable=SC2145,SC2154
log_msg() {
- local script_filename="$(basename $0)"
+ local script_filename
+ script_filename="$(basename "$0")"
logger -t "${script_filename%.*}" --id=$$ "${FLAGS_device} $@"
echo "$@"
}
@@ -253,9 +269,9 @@
local update_type="$1"
if [ "${update_type}" -eq "${UPDATE_NEEDED_OUT_OF_DATE}" ] ||
[ "${update_type}" -eq "${UPDATE_NEEDED_RECOVERY}" ]; then
- echo ${FLAGS_TRUE}
+ echo "${FLAGS_TRUE}"
else
- echo ${FLAGS_FALSE}
+ echo "${FLAGS_FALSE}"
fi
}
@@ -285,7 +301,7 @@
# Starting with the most significant values, compare them one
# by one until we find a difference or run out of values.
- for i in $(seq "$num_parts"); do
+ for i in $(seq "${num_parts}"); do
active_component="$(_canonicalize_ver_num "$1")"; shift
updater_component="$(_canonicalize_ver_num "$1")"; shift
if [ "${active_component}" -lt "${updater_component}" ]; then
@@ -300,7 +316,7 @@
if [ -z "${update_type}" ]; then
update_type="${UPDATE_NOT_NEEDED_UP_TO_DATE}"
- elif [ "${FLAGS_recovery}" -eq "${FLAGS_TRUE}" ] &&
+ elif [ "${FLAGS_recovery:-}" -eq "${FLAGS_TRUE}" ] &&
[ "${update_type}" = "${UPDATE_NOT_NEEDED_AHEAD_OF_DATE}" ]; then
update_type="${UPDATE_NEEDED_RECOVERY}"
fi
@@ -369,7 +385,7 @@
if [ -n "${extra_json_fields}" ]; then
json_fields="${json_fields}, ${extra_json_fields}"
fi
- echo "${sysfs_path}" '{'"${json_fields}"'}' >> $REPORT_FILE
+ echo "${sysfs_path}" '{'"${json_fields}"'}' >> "${REPORT_FILE}"
}
# Makes an untrusted string safe to include in a JSON object (by removing
@@ -379,7 +395,7 @@
}
# The version value that represents a device being in recovery mode.
-REPORT_VERSION_RECOVERY="(recovery)"
+export REPORT_VERSION_RECOVERY="(recovery)"
# Adds a report entry giving the initial state of a touch device.
# Parameters:
@@ -403,8 +419,8 @@
add_report_entry "${sysfs_path}" "${json_fields}" "${optional_extra_fields}"
}
-REPORT_RESULT_FAILURE="FAILURE"
-REPORT_RESULT_SUCCESS="SUCCESS"
+export REPORT_RESULT_FAILURE="FAILURE"
+export REPORT_RESULT_SUCCESS="SUCCESS"
# Adds a report entry describing the result of an update attempt.
# Parameters:
diff --git a/scripts/chromeos-touch-update-legacy.sh b/scripts/chromeos-touch-update-legacy.sh
index ff0cfcc..0f30064 100755
--- a/scripts/chromeos-touch-update-legacy.sh
+++ b/scripts/chromeos-touch-update-legacy.sh
@@ -6,7 +6,7 @@
# This isn't the exact copy that will be used in production, but it's better
# than pointing shellcheck at /dev/null.
-# shellcheck source=../../../scripts/lib/shflags/shflags
+# shellcheck source=../common/scripts/chromeos-touch-common.sh
. /opt/google/touch/scripts/chromeos-touch-common.sh
LOGGER_TAG="chromeos-touch-update"
diff --git a/scripts/chromeos-touch-update.sh b/scripts/chromeos-touch-update.sh
index 6accd7f..0e9639c 100755
--- a/scripts/chromeos-touch-update.sh
+++ b/scripts/chromeos-touch-update.sh
@@ -9,9 +9,6 @@
LOGGER_TAG="chromeos-touch-update"
-# Touch firmware and config updater for Chromebooks
-PIXART_VENDOR_ID="093A"
-
# Current firmware status returned by compare_fw_versions in vendor scripts
UP_TO_DATE="0"
# shellcheck disable=SC2034 # Unused variables pending TODO(b/129671661).
