Hi Kairui,
On Thu, 19 Aug 2021 19:39:27 +0800
Kairui Song <kasong(a)redhat.com> 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/kdumpctl b/kdumpctl
index f65f8e3d..ecfe3d77 100755
--- a/kdumpctl
+++ b/kdumpctl
[...]
@@ -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
^^
-q already suppresses any output to stdout.
No need to redirect it to /dev/null.
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
You could use grep -q instead of redirecting to /dev/null.
There's an other case in check_fs_modified and kdump-lib.sh. The
other findings from
git grep -E "grep.*>\s*/dev/null"
look ok to me. Maybe in general review all redirects to /dev/null and
check if they could be omitted with a quiet option.
Thanks
Philipp
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
}