On Thu, Aug 19, 2021 at 07:39:27PM +0800, Kairui Song wrote:
>As suggested by:
>https://github.com/koalaman/shellcheck/wiki/SC2181
>
>Signed-off-by: Kairui Song <kasong(a)redhat.com>
>---
> dracut-module-setup.sh | 41 ++++++---------
> kdumpctl | 112 +++++++++++++----------------------------
> mkdumprd | 40 ++++-----------
> 3 files changed, 61 insertions(+), 132 deletions(-)
>
>diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
>index 600bdc51..46c0087a 100755
>--- a/dracut-module-setup.sh
>+++ b/dracut-module-setup.sh
>@@ -255,8 +255,7 @@ kdump_static_ip() {
> _gateway="[$_gateway]"
> else
> _prefix=$(cut -d'/' -f2 <<< "$_ipaddr")
>- _netmask=$(cal_netmask_by_prefix "$_prefix"
"$_ipv6_flag")
>- if [[ "$?" -ne 0 ]]; then
>+ if ! _netmask=$(cal_netmask_by_prefix "$_prefix"
"$_ipv6_flag"); then
> derror "Failed to calculate netmask for $_ipaddr"
> exit 1
> fi
>@@ -358,10 +357,7 @@ kdump_setup_bridge() {
> [[ -e $_dev ]] || continue
> _kdumpdev=$_dev
> if kdump_is_bond "$_dev"; then
>- $(kdump_setup_bond "$_dev"
"$(get_nmcli_connection_show_cmd_by_ifname "$_dev")")
>- if [[ $? != 0 ]]; then
>- exit 1
>- fi
>+ $(kdump_setup_bond "$_dev"
"$(get_nmcli_connection_show_cmd_by_ifname "$_dev")") || exit 1
The above codes which was originally introduced by me breaks SC2091. I
think we can use "()" to replace "$()"
> elif kdump_is_team "$_dev"; then
> kdump_setup_team "$_dev"
> elif kdump_is_vlan "$_dev"; then
>@@ -419,9 +415,7 @@ kdump_setup_team() {
> echo " team=$_netdev:${_slaves%,}" >>
"${initdir}/etc/cmdline.d/44team.conf"
> #Buggy version teamdctl outputs to stderr!
> #Try to use the latest version of teamd.
>- teamdctl "$_netdev" config dump >
"${initdir}/tmp/$$-$_netdev.conf"
>- if [[ $? -ne 0 ]]
>- then
>+ if ! teamdctl "$_netdev" config dump >
"${initdir}/tmp/$$-$_netdev.conf"; then
> derror "teamdctl failed."
> exit 1
> fi
>@@ -441,10 +435,7 @@ kdump_setup_vlan() {
> derror "Vlan over bridge is not supported!"
> exit 1
> elif kdump_is_bond "$_phydev"; then
>- $(kdump_setup_bond "$_phydev"
"$(get_nmcli_connection_show_cmd_by_ifname "$_phydev")")
>- if [[ $? != 0 ]]; then
>- exit 1
>- fi
>+ $(kdump_setup_bond "$_phydev"
"$(get_nmcli_connection_show_cmd_by_ifname "$_phydev")") || exit 1
Here too.
> echo " vlan=$(kdump_setup_ifname "$_netdev"):$_phydev"
> "${initdir}/etc/cmdline.d/43vlan.conf"
> else
> _kdumpdev="$(kdump_setup_ifname "$_phydev")"
>@@ -515,8 +506,8 @@ kdump_setup_znet() {
>
> kdump_get_ip_route()
> {
>- local _route=$(/sbin/ip -o route get to "$1" 2>&1)
>- if [[ $? != 0 ]]; then
>+ local _route
>+ if ! _route=$(/sbin/ip -o route get to "$1" 2>&1); then
> derror "Bad kdump network destination: $1"
> exit 1
> fi
>@@ -560,8 +551,7 @@ kdump_install_net() {
> _znet_netdev=$(find_online_znet_device)
> if [[ -n "$_znet_netdev" ]]; then
> _nm_show_cmd_znet=$(get_nmcli_connection_show_cmd_by_ifname
"$_znet_netdev")
>- $(kdump_setup_znet "$_znet_netdev"
"$_nm_show_cmd_znet")
>- if [[ $? != 0 ]]; then
>+ if ! $(kdump_setup_znet "$_znet_netdev"
"$_nm_show_cmd_znet"); then
And here.
> derror "Failed to set up znet"
> exit 1
> fi
>@@ -591,10 +581,7 @@ kdump_install_net() {
> if kdump_is_bridge "$_netdev"; then
> kdump_setup_bridge "$_netdev"
> elif kdump_is_bond "$_netdev"; then
>- $(kdump_setup_bond "$_netdev" "$_nm_show_cmd")
>- if [[ $? != 0 ]]; then
>- exit 1
>- fi
>+ $(kdump_setup_bond "$_netdev" "$_nm_show_cmd") || exit
1
> elif kdump_is_team "$_netdev"; then
> kdump_setup_team "$_netdev"
> elif kdump_is_vlan "$_netdev"; then
>@@ -836,8 +823,10 @@ kdump_setup_iscsi_device() {
> fi
>
> # Setup initator
>- initiator_str=$(kdump_get_iscsi_initiator)
>- [[ $? -ne "0" ]] && derror "Failed to get initiator
name" && return 1
>+ if ! initiator_str=$(kdump_get_iscsi_initiator); then
>+ derror "Failed to get initiator name"
>+ return 1
>+ fi
>
> # If initiator details do not exist already, append.
> if ! grep -q "$initiator_str" "$netroot_conf"; then
>@@ -877,8 +866,7 @@ get_alias() {
> for ip in $ips
> do
> # in /etc/hosts, alias can come at the 2nd column
>- entries=$(grep "$ip" /etc/hosts | awk '{ $1="";
print $0 }')
>- if [[ $? -eq 0 ]]; then
>+ if entries=$(grep "$ip" /etc/hosts | awk '{
$1=""; print $0 }'); then
> alias_set="$alias_set $entries"
> fi
> done
>@@ -1003,8 +991,7 @@ kdump_install_random_seed() {
> kdump_install_systemd_conf() {
> # Kdump turns out to require longer default systemd mount timeout
> # than 1st kernel(90s by default), we use default 300s for kdump.
>- grep -r "^[[:space:]]*DefaultTimeoutStartSec="
"${initdir}/etc/systemd/system.conf"* &>/dev/null
>- if [[ $? -ne 0 ]]; then
>+ if ! grep -q -r "^[[:space:]]*DefaultTimeoutStartSec="
"${initdir}/etc/systemd/system.conf"*; then
> mkdir -p "${initdir}/etc/systemd/system.conf.d"
> echo "[Manager]" >
"${initdir}/etc/systemd/system.conf.d/kdump.conf"
> echo "DefaultTimeoutStartSec=300s" >>
"${initdir}/etc/systemd/system.conf.d/kdump.conf"
>diff --git a/kdumpctl b/kdumpctl
>index f65f8e3d..ecfe3d77 100755
>--- a/kdumpctl
>+++ b/kdumpctl
>@@ -37,8 +37,7 @@ fi
> . /lib/kdump/kdump-logger.sh
>
> #initiate the kdump logger
>-dlog_init
>-if [[ $? -ne 0 ]]; then
>+if ! dlog_init; then
> echo "failed to initiate the kdump logger."
> exit 1
> fi
>@@ -47,8 +46,7 @@ single_instance_lock()
> {
> local rc timeout=5
>
>- exec 9>/var/lock/kdump
>- if [[ $? -ne 0 ]]; then
>+ if ! exec 9>/var/lock/kdump; then
> derror "Create file lock failed"
> exit 1
> fi
>@@ -80,8 +78,7 @@ save_core()
>
> mkdir -p "$coredir"
> ddebug "cp --sparse=always /proc/vmcore $coredir/vmcore-incomplete"
>- cp --sparse=always /proc/vmcore "$coredir/vmcore-incomplete"
>- if [[ $? == 0 ]]; then
>+ if cp --sparse=always /proc/vmcore "$coredir/vmcore-incomplete";
then
> mv "$coredir/vmcore-incomplete" "$coredir/vmcore"
> dinfo "saved a vmcore to $coredir"
> else
>@@ -95,8 +92,7 @@ save_core()
> ddebug "makedumpfile --dump-dmesg $coredir/vmcore
$coredir/dmesg"
> makedumpfile --dump-dmesg "$coredir/vmcore"
"$coredir/dmesg" >/dev/null 2>&1
> ddebug "dumpoops -d $coredir/dmesg"
>- dumpoops -d "$coredir/dmesg" >/dev/null 2>&1
>- if [[ $? == 0 ]]; then
>+ if dumpoops -d "$coredir/dmesg" >/dev/null 2>&1;
then
> dinfo "kernel oops has been collected by abrt tool"
> fi
> fi
>@@ -121,8 +117,7 @@ check_earlykdump_is_enabled()
> rebuild_kdump_initrd()
> {
> ddebug "rebuild kdump initrd: $MKDUMPRD $TARGET_INITRD
$KDUMP_KERNELVER"
>- $MKDUMPRD "$TARGET_INITRD" "$KDUMP_KERNELVER"
>- if [[ $? != 0 ]]; then
>+ if ! $MKDUMPRD "$TARGET_INITRD" "$KDUMP_KERNELVER"; then
> derror "mkdumprd: failed to make kdump initrd"
> return 1
> fi
>@@ -184,8 +179,7 @@ backup_default_initrd()
> dinfo "Backing up $DEFAULT_INITRD before rebuild."
> # save checksum to verify before restoring
> sha1sum "$DEFAULT_INITRD" >
"$INITRD_CHECKSUM_LOCATION"
>- cp "$DEFAULT_INITRD" "$DEFAULT_INITRD_BAK"
>- if [[ $? -ne 0 ]]; then
>+ if ! cp "$DEFAULT_INITRD" "$DEFAULT_INITRD_BAK";
then
> dwarn "WARNING: failed to backup $DEFAULT_INITRD."
> rm -f "$DEFAULT_INITRD_BAK"
> fi
>@@ -210,8 +204,7 @@ restore_default_initrd()
> dwarn "WARNING: checksum mismatch! Can't restore
original initrd.."
> else
> rm -f $INITRD_CHECKSUM_LOCATION
>- mv "$DEFAULT_INITRD_BAK"
"$DEFAULT_INITRD"
>- if [[ $? -eq 0 ]]; then
>+ if mv "$DEFAULT_INITRD_BAK"
"$DEFAULT_INITRD"; then
> derror "Restoring original initrd as fadump mode
is disabled."
> sync
> fi
>@@ -308,8 +301,7 @@ get_pcs_cluster_modified_files()
>
> setup_initrd()
> {
>- prepare_kdump_bootinfo
>- if [[ $? -ne 0 ]]; then
>+ if ! prepare_kdump_bootinfo; then
> derror "failed to prepare for kdump bootinfo."
> return 1
> fi
>@@ -372,8 +364,7 @@ check_files_modified()
> files="$files
/lib/modules/$KDUMP_KERNELVER/modules.dep"
> fi
> for _module in $EXTRA_MODULES; do
>- _module_file="$(modinfo --set-version
"$KDUMP_KERNELVER" --filename "$_module" 2>/dev/null)"
>- if [[ $? -eq 0 ]]; then
>+ if _module_file="$(modinfo --set-version
"$KDUMP_KERNELVER" --filename "$_module" 2>/dev/null)"; then
> files="$files $_module_file"
> for _dep_modules in $(modinfo -F depends
"$_module" | tr ',' ' '); do
> files="$files $(modinfo --set-version
"$KDUMP_KERNELVER" --filename "$_dep_modules" 2>/dev/null)"
>@@ -389,8 +380,7 @@ check_files_modified()
>
> # HOOKS is mandatory and need to check the modification time
> files="$files $HOOKS"
>- check_exist "$files" && check_executable
"$EXTRA_BINS"
>- [[ $? -ne 0 ]] && return 2
>+ check_exist "$files" && check_executable
"$EXTRA_BINS" || return 2
>
> for file in $files; do
> if [[ -e "$file" ]]; then
>@@ -455,7 +445,7 @@ check_drivers_modified()
> # Skip deprecated/invalid driver name or built-in module
> _module_name=$(modinfo --set-version "$KDUMP_KERNELVER" -F
name "$_driver" 2>/dev/null)
> _module_filename=$(modinfo --set-version "$KDUMP_KERNELVER"
-n "$_driver" 2>/dev/null)
>- if [[ $? -ne 0 ]] || [[ -z "$_module_name" ]] || [[
"$_module_filename" = *"(builtin)"* ]]; then
>+ if [[ -z "$_module_name" ]] || [[ -z
"$_module_filename" ]] || [[ "$_module_filename" =
*"(builtin)"* ]]; then
> continue
> fi
> if ! [[ " $_old_drivers " == *" $_module_name "*
]]; then
>@@ -505,8 +495,7 @@ check_fs_modified()
>
> # if --mount argument present then match old and new target, mount
> # point and file system. If any of them mismatches then rebuild
>- echo "$_dracut_args" | grep "\-\-mount" &>
/dev/null
>- if [[ $? -eq 0 ]];then
>+ if echo "$_dracut_args" | grep "\-\-mount" &>
/dev/null; then
> # shellcheck disable=SC2046
> set -- $(echo "$_dracut_args" | awk -F "--mount
'" '{print $2}' | cut -d' ' -f1,2,3)
> _old_dev=$1
>@@ -558,11 +547,7 @@ check_rebuild()
> local force_rebuild force_no_rebuild
> local ret system_modified="0"
>
>- setup_initrd
>-
>- if [[ $? -ne 0 ]]; then
>- return 1
>- fi
>+ setup_initrd || return 1
>
> force_no_rebuild=$(kdump_get_conf_val force_no_rebuild)
> force_no_rebuild=${force_no_rebuild:-0}
>@@ -761,8 +746,7 @@ check_and_wait_network_ready()
>
> # if server removes the authorized_keys or, no
/root/.ssh/kdump_id_rsa
> ddebug "$errmsg"
>- echo "$errmsg" | grep -q "Permission denied\|No such
file or directory\|Host key verification failed" &> /dev/null
>- if [[ $? -eq 0 ]]; then
>+ if echo "$errmsg" | grep -q "Permission denied\|No such
file or directory\|Host key verification failed" &> /dev/null; then
> derror "Could not create $DUMP_TARGET:$SAVE_PATH, you
probably need to run \"kdumpctl propagate\""
> return 1
> fi
>@@ -788,16 +772,11 @@ check_and_wait_network_ready()
> check_ssh_target()
> {
> check_and_wait_network_ready
>- if [[ $? -ne 0 ]]; then
>- return 1
>- fi
>- return 0
> }
>
> propagate_ssh_key()
> {
>- check_ssh_config
>- if [[ $? -ne 0 ]]; then
>+ if ! check_ssh_config; then
> derror "No ssh config specified in $KDUMP_CONFIG_FILE. Can't
propagate"
> exit 1
> fi
>@@ -904,8 +883,7 @@ local_fs_dump_target()
> {
> local _target
>
>- _target=$(egrep "^ext[234]|^xfs|^btrfs|^minix" /etc/kdump.conf)
>- if [[ $? -eq 0 ]]; then
>+ if _target=$(grep -E "^ext[234]|^xfs|^btrfs|^minix"
/etc/kdump.conf); then
> echo "$_target" | awk '{print $2}'
> fi
> }
>@@ -967,8 +945,7 @@ check_fence_kdump_config()
> return 1
> fi
> # node can be ipaddr
>- echo "$ipaddrs " | grep "$node " > /dev/null
>- if [[ $? -eq 0 ]]; then
>+ if echo "$ipaddrs " | grep "$node " >
/dev/null; then
> derror "Option fence_kdump_nodes cannot contain
$node"
> return 1
> fi
>@@ -1062,14 +1039,12 @@ check_final_action_config()
>
> start()
> {
>- check_dump_feasibility
>- if [[ $? -ne 0 ]]; then
>+ if ! check_dump_feasibility; then
> derror "Starting kdump: [FAILED]"
> return 1
> fi
>
>- check_config
>- if [[ $? -ne 0 ]]; then
>+ if ! check_config; then
> derror "Starting kdump: [FAILED]"
> return 1
> fi
>@@ -1078,14 +1053,12 @@ start()
> selinux_relabel
> fi
>
>- save_raw
>- if [[ $? -ne 0 ]]; then
>+ if ! save_raw; then
> derror "Starting kdump: [FAILED]"
> return 1
> fi
>
>- check_current_status
>- if [[ $? == 0 ]]; then
>+ if check_current_status; then
> dwarn "Kdump already running: [WARNING]"
> return 0
> fi
>@@ -1097,14 +1070,12 @@ start()
> fi
> fi
>
>- check_rebuild
>- if [[ $? != 0 ]]; then
>+ if ! check_rebuild; then
> derror "Starting kdump: [FAILED]"
> return 1
> fi
>
>- start_dump
>- if [[ $? != 0 ]]; then
>+ if ! start_dump; then
> derror "Starting kdump: [FAILED]"
> return 1
> fi
>@@ -1114,8 +1085,7 @@ start()
>
> reload()
> {
>- check_current_status
>- if [[ $? -ne 0 ]]; then
>+ if ! check_current_status; then
> dwarn "Kdump was not running: [WARNING]"
> fi
>
>@@ -1123,24 +1093,20 @@ reload()
> reload_fadump
> return $?
> else
>- stop_kdump
>- fi
>-
>- if [[ $? -ne 0 ]]; then
>- derror "Stopping kdump: [FAILED]"
>- return 1
>+ if ! stop_kdump; then
>+ derror "Stopping kdump: [FAILED]"
>+ return 1
>+ fi
> fi
>
> dinfo "Stopping kdump: [OK]"
>
>- setup_initrd
>- if [[ $? -ne 0 ]]; then
>+ if ! setup_initrd; then
> derror "Starting kdump: [FAILED]"
> return 1
> fi
>
>- start_dump
>- if [[ $? -ne 0 ]]; then
>+ if ! start_dump; then
> derror "Starting kdump: [FAILED]"
> return 1
> fi
>@@ -1168,6 +1134,7 @@ stop_kdump()
> $KEXEC -p -u
> fi
>
>+ # shellcheck disable=SC2181
> if [[ $? != 0 ]]; then
> derror "kexec: failed to unload kdump kernel"
> return 1
>@@ -1179,16 +1146,14 @@ stop_kdump()
>
> reload_fadump()
> {
>- echo 1 > $FADUMP_REGISTER_SYS_NODE
>- if [[ $? == 0 ]]; then
>+ if echo 1 > $FADUMP_REGISTER_SYS_NODE; then
> dinfo "fadump: re-registered successfully"
> return 0
> else
> # FADump could fail on older kernel where re-register
> # support is not enabled. Try stop/start from userspace
> # to handle such scenario.
>- stop_fadump
>- if [[ $? == 0 ]]; then
>+ if stop_fadump; then
> start_fadump
> return $?
> fi
>@@ -1205,6 +1170,7 @@ stop()
> stop_kdump
> fi
>
>+ # shellcheck disable=SC2181
> if [[ $? != 0 ]]; then
> derror "Stopping kdump: [FAILED]"
> return 1
>@@ -1215,10 +1181,7 @@ stop()
> }
>
> rebuild() {
>- check_config
>- if [[ $? -ne 0 ]]; then
>- return 1
>- fi
>+ check_config || return 1
>
> if check_ssh_config; then
> if ! check_ssh_target; then
>@@ -1226,10 +1189,7 @@ rebuild() {
> fi
> fi
>
>- setup_initrd
>- if [[ $? -ne 0 ]]; then
>- return 1
>- fi
>+ setup_initrd || return 1
>
> dinfo "Rebuilding $TARGET_INITRD"
> rebuild_initrd
>diff --git a/mkdumprd b/mkdumprd
>index 9800934e..a2c6ed29 100644
>--- a/mkdumprd
>+++ b/mkdumprd
>@@ -17,8 +17,7 @@ fi
> export IN_KDUMP=1
>
> #initiate the kdump logger
>-dlog_init
>-if [[ $? -ne 0 ]]; then
>+if ! dlog_init; then
> echo "failed to initiate the kdump logger."
> exit 1
> fi
>@@ -114,18 +113,12 @@ mkdir_save_path_ssh()
> {
> local _opt _dir
> _opt=(-i "$SSH_KEY_LOCATION" -o BatchMode=yes -o
StrictHostKeyChecking=yes)
>- ssh -qn "${_opt[@]}" "$1" mkdir -p "$SAVE_PATH"
2>&1 > /dev/null
>- _ret=$?
>- if [[ $_ret -ne 0 ]]; then
>+ ssh -qn "${_opt[@]}" "$1" mkdir -p "$SAVE_PATH"
&>/dev/null || \
> perror_exit "mkdir failed on $1:$SAVE_PATH"
>- fi
>
>- #check whether user has write permission on $1:$SAVE_PATH
>- _dir=$(ssh -qn "${_opt[@]}" "$1" mktemp -dqp
"$SAVE_PATH" 2>/dev/null)
>- _ret=$?
>- if [[ $_ret -ne 0 ]]; then
>+ # check whether user has write permission on $1:$SAVE_PATH
>+ _dir=$(ssh -qn "${_opt[@]}" "$1" mktemp -dqp
"$SAVE_PATH" 2>/dev/null) || \
> perror_exit "Could not create temporary directory on $1:$SAVE_PATH.
Make sure user has write permission on destination"
>- fi
> ssh -qn "${_opt[@]}" "$1" rmdir "$_dir"
>
> return 0
>@@ -162,11 +155,7 @@ check_size() {
> ;;
> *)
> return
>- esac
>-
>- if [[ $? -ne 0 ]]; then
>- perror_exit "Check dump target size failed"
>- fi
>+ esac || perror_exit "Check dump target size failed"
>
> if [[ "$avail" -lt "$memtotal" ]]; then
> dwarn "Warning: There might not be enough space to save a
vmcore."
>@@ -227,8 +216,7 @@ check_user_configured_target()
> if [[ -n "$_mnt" ]]; then
> if ! is_mounted "$_mnt"; then
> if [[ $_opt = *",noauto"* ]]; then
>- mount "$_mnt"
>- [[ $? -ne 0 ]] && mount_failure "$_target"
"$_mnt" "$_fstype"
>+ mount "$_mnt" || mount_failure "$_target"
"$_mnt" "$_fstype"
> _mounted=$_mnt
> else
> perror_exit "Dump target \"$_target\" is neither
mounted nor configured as \"noauto\""
>@@ -237,8 +225,7 @@ check_user_configured_target()
> else
> _mnt=$MKDUMPRD_TMPMNT
> mkdir -p "$_mnt"
>- mount "$_target" "$_mnt" -t "$_fstype" -o
defaults
>- [[ $? -ne 0 ]] && mount_failure "$_target" ""
"$_fstype"
>+ mount "$_target" "$_mnt" -t "$_fstype" -o
defaults || mount_failure "$_target" "" "$_fstype"
> _mounted=$_mnt
> fi
>
>@@ -283,11 +270,9 @@ verify_core_collector() {
> }
>
> add_mount() {
>- local _mnt=$(to_mount "$@")
>+ local _mnt
>
>- if [[ $? -ne 0 ]]; then
>- exit 1
>- fi
>+ _mnt=$(to_mount "$@") || exit 1
>
> add_dracut_mount "$_mnt"
> }
>@@ -349,15 +334,12 @@ is_unresettable()
> #return true if resettable
> check_resettable()
> {
>- local _ret _target _override_resettable
>+ local _target _override_resettable
>
> _override_resettable=$(kdump_get_conf_val override_resettable)
> OVERRIDE_RESETTABLE=${_override_resettable:-$OVERRIDE_RESETTABLE}
>
>- for_each_block_target is_unresettable
>- _ret=$?
>-
>- [[ $_ret -eq 0 ]] && return
>+ for_each_block_target is_unresettable && return
>
> return 1
> }
>--
>2.31.1
>_______________________________________________
>kexec mailing list -- kexec(a)lists.fedoraproject.org
>To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
>List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org
>Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
--
Best regards,
Coiby
Good idea, I didn't change this part since I'm not entirely sure about
the context and if this is safe, I can update the code then.
--
Best Regards,
Kairui Song