From f6d6b60a6a51723799d2c8303e7f2cefb3fd70e8 Mon Sep 17 00:00:00 2001 From: Tao Liu Date: Tue, 9 Nov 2021 21:35:45 +0800 Subject: [PATCH] bash scripts: fix redundant exit code check upstream: fedora resolves: bz2003832 conflict: none commit a4648fc851be56d141fd43c8ade1a61c4069f47e Author: Kairui Song Date: Wed Sep 8 17:23:16 2021 +0800 bash scripts: fix redundant exit code check As suggested by: https://github.com/koalaman/shellcheck/wiki/SC2181 Signed-off-by: Kairui Song Acked-by: Philipp Rudo Signed-off-by: Tao Liu --- 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 07a96e5..7c51e01 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 @@ -359,10 +358,7 @@ kdump_setup_bridge() { _dev=${_dev##*/} _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 elif kdump_is_team "$_dev"; then kdump_setup_team "$_dev" elif kdump_is_vlan "$_dev"; then @@ -420,9 +416,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 @@ -442,10 +436,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 echo " vlan=$(kdump_setup_ifname "$_netdev"):$_phydev" > "${initdir}/etc/cmdline.d/43vlan.conf" else _kdumpdev="$(kdump_setup_ifname "$_phydev")" @@ -516,8 +507,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 @@ -561,8 +552,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 derror "Failed to set up znet" exit 1 fi @@ -592,10 +582,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 @@ -837,8 +824,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 @@ -878,8 +867,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 @@ -1004,8 +992,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 a3c06b3..0544a7d 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 -q "\-\-mount" - if [[ $? -eq 0 ]];then + if echo "$_dracut_args" | grep -q "\-\-mount"; 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} @@ -730,8 +715,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" - if [[ $? -eq 0 ]]; then + if echo "$errmsg" | grep -q "Permission denied\|No such file or directory\|Host key verification failed"; then derror "Could not create $DUMP_TARGET:$SAVE_PATH, you probably need to run \"kdumpctl propagate\"" return 1 fi @@ -757,16 +741,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 @@ -873,8 +852,7 @@ local_fs_dump_target() { local _target - _target=$(grep -E "^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 } @@ -936,8 +914,7 @@ check_fence_kdump_config() return 1 fi # node can be ipaddr - echo "$ipaddrs " | grep -q "$node " - if [[ $? -eq 0 ]]; then + if echo "$ipaddrs " | grep -q "$node "; then derror "Option fence_kdump_nodes cannot contain $node" return 1 fi @@ -1031,14 +1008,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 @@ -1047,14 +1022,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 @@ -1066,14 +1039,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 @@ -1083,8 +1054,7 @@ start() reload() { - check_current_status - if [[ $? -ne 0 ]]; then + if ! check_current_status; then dwarn "Kdump was not running: [WARNING]" fi @@ -1092,24 +1062,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 @@ -1137,6 +1103,7 @@ stop_kdump() $KEXEC -p -u fi + # shellcheck disable=SC2181 if [[ $? != 0 ]]; then derror "kexec: failed to unload kdump kernel" return 1 @@ -1148,16 +1115,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 @@ -1174,6 +1139,7 @@ stop() stop_kdump fi + # shellcheck disable=SC2181 if [[ $? != 0 ]]; then derror "Stopping kdump: [FAILED]" return 1 @@ -1184,10 +1150,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 @@ -1195,10 +1158,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 b5cc39e..b75bbdd 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,7 +334,7 @@ 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} @@ -357,10 +342,7 @@ check_resettable() perror_exit "override_resettable value '$OVERRIDE_RESETTABLE' is invalid" fi - for_each_block_target is_unresettable - _ret=$? - - [[ $_ret -eq 0 ]] && return + for_each_block_target is_unresettable && return return 1 }