Re: [PATCH v2 28/49] bash scripts: fix redundant exit code check

1 Sep 2021

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@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@lists.fedoraproject.org
To unsubscribe send an email to kexec-leave@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

    

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

Re: [PATCH v2 28/49] bash scripts: fix redundant exit code check

Signed-off-by: Kairui Song kasong@redhat.com

}