* [PATCH] common: cleanups after common/rc split
@ 2016-12-01 4:22 Eryu Guan
2016-12-02 3:29 ` Dave Chinner
0 siblings, 1 reply; 5+ messages in thread
From: Eryu Guan @ 2016-12-01 4:22 UTC (permalink / raw)
To: fstests; +Cc: Eryu Guan
Fix code style issues after the common/rc split, e.g. use tab
indention, fix if-then-else format, fix while-do and for-do format
and various whitespace issues. No functional changes.
Signed-off-by: Eryu Guan <eguan@redhat.com>
---
This is a follow-up patch to Dave's "make common/rc easier to manage" patchset.
common/btrfs | 87 ++++++-------
common/xfs | 404 +++++++++++++++++++++++++++++------------------------------
2 files changed, 239 insertions(+), 252 deletions(-)
diff --git a/common/btrfs b/common/btrfs
index 7cc4f5d..ab6497d 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -68,53 +68,49 @@ _require_btrfs_fs_feature()
_check_btrfs_filesystem()
{
- device=$1
-
- # If type is set, we're mounted
- type=`_fs_type $device`
- ok=1
-
- if [ "$type" = "$FSTYP" ]
- then
- # mounted ...
- mountpoint=`_umount_or_remount_ro $device`
- fi
-
- btrfsck $device >$tmp.fsck 2>&1
- if [ $? -ne 0 ]
- then
- echo "_check_btrfs_filesystem: filesystem on $device is inconsistent (see $seqres.full)"
-
- echo "_check_btrfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
- echo "*** fsck.$FSTYP output ***" >>$seqres.full
- cat $tmp.fsck >>$seqres.full
- echo "*** end fsck.$FSTYP output" >>$seqres.full
-
- ok=0
- fi
- rm -f $tmp.fsck
-
- if [ $ok -eq 0 ]
- then
- echo "*** mount output ***" >>$seqres.full
- _mount >>$seqres.full
- echo "*** end mount output" >>$seqres.full
- elif [ "$type" = "$FSTYP" ]
- then
- # was mounted ...
- _mount_or_remount_rw "$MOUNT_OPTIONS" $device $mountpoint
- ok=$?
- fi
-
- if [ $ok -eq 0 ]; then
- status=1
- if [ "$iam" != "check" ]; then
- exit 1
+ device=$1
+
+ # If type is set, we're mounted
+ type=`_fs_type $device`
+ ok=1
+
+ if [ "$type" = "$FSTYP" ]; then
+ # mounted ...
+ mountpoint=`_umount_or_remount_ro $device`
fi
- return 1
- fi
- return 0
+ btrfsck $device >$tmp.fsck 2>&1
+ if [ $? -ne 0 ]; then
+ echo "_check_btrfs_filesystem: filesystem on $device is inconsistent (see $seqres.full)"
+
+ echo "_check_btrfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
+ echo "*** fsck.$FSTYP output ***" >>$seqres.full
+ cat $tmp.fsck >>$seqres.full
+ echo "*** end fsck.$FSTYP output" >>$seqres.full
+
+ ok=0
+ fi
+ rm -f $tmp.fsck
+
+ if [ $ok -eq 0 ]; then
+ echo "*** mount output ***" >>$seqres.full
+ _mount >>$seqres.full
+ echo "*** end mount output" >>$seqres.full
+ elif [ "$type" = "$FSTYP" ]; then
+ # was mounted ...
+ _mount_or_remount_rw "$MOUNT_OPTIONS" $device $mountpoint
+ ok=$?
+ fi
+
+ if [ $ok -eq 0 ]; then
+ status=1
+ if [ "$iam" != "check" ]; then
+ exit 1
+ fi
+ return 1
+ fi
+
+ return 0
}
_require_btrfs_dev_del_by_devid()
@@ -332,4 +328,3 @@ _reload_btrfs_ko()
modprobe -r btrfs || _fail "btrfs unload failed"
modprobe btrfs || _fail "btrfs load failed"
}
-
diff --git a/common/xfs b/common/xfs
index 0dde83f..53cd4de 100644
--- a/common/xfs
+++ b/common/xfs
@@ -135,44 +135,43 @@ _scratch_mkfs_xfs()
# maintain the current verification levels.
_xfs_check()
{
- OPTS=" "
- DBOPTS=" "
- USAGE="Usage: xfs_check [-fsvV] [-l logdev] [-i ino]... [-b bno]... special"
-
- while getopts "b:fi:l:stvV" c
- do
- case $c in
- s) OPTS=$OPTS"-s ";;
- t) OPTS=$OPTS"-t ";;
- v) OPTS=$OPTS"-v ";;
- i) OPTS=$OPTS"-i "$OPTARG" ";;
- b) OPTS=$OPTS"-b "$OPTARG" ";;
- f) DBOPTS=$DBOPTS" -f";;
- l) DBOPTS=$DBOPTS" -l "$OPTARG" ";;
- V) $XFS_DB_PROG -p xfs_check -V
- return $?
- ;;
- esac
- done
- set -- extra $@
- shift $OPTIND
- case $# in
- 1) ${XFS_DB_PROG}${DBOPTS} -F -i -p xfs_check -c "check$OPTS" $1
- status=$?
- ;;
- 2) echo $USAGE 1>&1
- status=2
- ;;
- esac
- return $status
+ OPTS=" "
+ DBOPTS=" "
+ USAGE="Usage: xfs_check [-fsvV] [-l logdev] [-i ino]... [-b bno]... special"
+
+ while getopts "b:fi:l:stvV" c; do
+ case $c in
+ s) OPTS=$OPTS"-s ";;
+ t) OPTS=$OPTS"-t ";;
+ v) OPTS=$OPTS"-v ";;
+ i) OPTS=$OPTS"-i "$OPTARG" ";;
+ b) OPTS=$OPTS"-b "$OPTARG" ";;
+ f) DBOPTS=$DBOPTS" -f";;
+ l) DBOPTS=$DBOPTS" -l "$OPTARG" ";;
+ V) $XFS_DB_PROG -p xfs_check -V
+ return $?
+ ;;
+ esac
+ done
+ set -- extra $@
+ shift $OPTIND
+ case $# in
+ 1) ${XFS_DB_PROG}${DBOPTS} -F -i -p xfs_check -c "check$OPTS" $1
+ status=$?
+ ;;
+ 2) echo $USAGE 1>&1
+ status=2
+ ;;
+ esac
+ return $status
}
_scratch_xfs_db_options()
{
- SCRATCH_OPTIONS=""
- [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
- SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
- echo $SCRATCH_OPTIONS $* $SCRATCH_DEV
+ SCRATCH_OPTIONS=""
+ [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+ SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
+ echo $SCRATCH_OPTIONS $* $SCRATCH_DEV
}
_scratch_xfs_db()
@@ -182,10 +181,10 @@ _scratch_xfs_db()
_scratch_xfs_logprint()
{
- SCRATCH_OPTIONS=""
- [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
- SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
- $XFS_LOGPRINT_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
+ SCRATCH_OPTIONS=""
+ [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+ SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
+ $XFS_LOGPRINT_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
}
_test_xfs_logprint()
@@ -198,23 +197,23 @@ _test_xfs_logprint()
_scratch_xfs_check()
{
- SCRATCH_OPTIONS=""
- [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
- SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
- [ "$LARGE_SCRATCH_DEV" = yes ] && \
- SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
- _xfs_check $SCRATCH_OPTIONS $* $SCRATCH_DEV
+ SCRATCH_OPTIONS=""
+ [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+ SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
+ [ "$LARGE_SCRATCH_DEV" = yes ] && \
+ SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
+ _xfs_check $SCRATCH_OPTIONS $* $SCRATCH_DEV
}
_scratch_xfs_repair()
{
- SCRATCH_OPTIONS=""
- [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
- SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
- [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
- SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV"
- [ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
- $XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
+ SCRATCH_OPTIONS=""
+ [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+ SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
+ [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
+ SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV"
+ [ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
+ $XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
}
# this test requires the projid32bit feature to be available in mkfs.xfs.
@@ -309,8 +308,7 @@ _require_xfs_sparse_inodes()
# check that xfs_db supports a specific command
_require_xfs_db_command()
{
- if [ $# -ne 1 ]
- then
+ if [ $# -ne 1 ]; then
echo "Usage: _require_xfs_db_command command" 1>&2
exit 1
fi
@@ -323,164 +321,157 @@ _require_xfs_db_command()
# run xfs_check and friends on a FS.
_check_xfs_filesystem()
{
- if [ $# -ne 3 ]
- then
- echo "Usage: _check_xfs_filesystem device <logdev>|none <rtdev>|none" 1>&2
- exit 1
- fi
-
- extra_mount_options=""
- extra_log_options=""
- extra_options=""
- device=$1
- if [ -f $device ];then
- extra_options="-f"
- fi
-
- if [ "$2" != "none" ]; then
- extra_log_options="-l$2"
- extra_mount_options="-ologdev=$2"
- fi
-
- if [ "$3" != "none" ]; then
- extra_rt_options="-r$3"
- extra_mount_options=$extra_mount_options" -ortdev=$3"
- fi
- extra_mount_options=$extra_mount_options" $MOUNT_OPTIONS"
-
- [ "$FSTYP" != xfs ] && return 0
-
- type=`_fs_type $device`
- ok=1
-
- if [ "$type" = "xfs" ]
- then
- if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
- "$XFS_SCRUB_PROG" $scrubflag -vd $device >>$seqres.full
- if [ $? -ne 0 ]; then
- echo "filesystem on $device failed scrub (see $seqres.full)"
- ok=0
- fi
- fi
- # mounted ...
- mountpoint=`_umount_or_remount_ro $device`
- fi
-
- $XFS_LOGPRINT_PROG -t $extra_log_options $device 2>&1 \
- | tee $tmp.logprint | grep -q "<CLEAN>"
- if [ $? -ne 0 -a "$HOSTOS" = "Linux" ]
- then
- echo "_check_xfs_filesystem: filesystem on $device has dirty log (see $seqres.full)"
-
- echo "_check_xfs_filesystem: filesystem on $device has dirty log" >>$seqres.full
- echo "*** xfs_logprint -t output ***" >>$seqres.full
- cat $tmp.logprint >>$seqres.full
- echo "*** end xfs_logprint output" >>$seqres.full
-
- ok=0
- fi
-
- # xfs_check runs out of memory on large files, so even providing the test
- # option (-t) to avoid indexing the free space trees doesn't make it pass on
- # large filesystems. Avoid it.
- if [ "$LARGE_SCRATCH_DEV" != yes ]; then
- _xfs_check $extra_log_options $device 2>&1 |\
- _fix_malloc >$tmp.fs_check
- fi
- if [ -s $tmp.fs_check ]
- then
- echo "_check_xfs_filesystem: filesystem on $device is inconsistent (c) (see $seqres.full)"
-
- echo "_check_xfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
- echo "*** xfs_check output ***" >>$seqres.full
- cat $tmp.fs_check >>$seqres.full
- echo "*** end xfs_check output" >>$seqres.full
-
- ok=0
- fi
-
- $XFS_REPAIR_PROG -n $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
- if [ $? -ne 0 ]
- then
- echo "_check_xfs_filesystem: filesystem on $device is inconsistent (r) (see $seqres.full)"
-
- echo "_check_xfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
- echo "*** xfs_repair -n output ***" >>$seqres.full
- cat $tmp.repair | _fix_malloc >>$seqres.full
- echo "*** end xfs_repair output" >>$seqres.full
-
- ok=0
- fi
- rm -f $tmp.fs_check $tmp.logprint $tmp.repair
-
- # Optionally test the index rebuilding behavior.
- if [ -n "$TEST_XFS_REPAIR_REBUILD" ]; then
- $XFS_REPAIR_PROG $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
- if [ $? -ne 0 ]; then
- echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild) (see $seqres.full)"
-
- echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild)" >>$seqres.full
- echo "*** xfs_repair output ***" >>$seqres.full
- cat $tmp.repair | _fix_malloc >>$seqres.full
- echo "*** end xfs_repair output" >>$seqres.full
-
- ok=0
- fi
- rm -f $tmp.repair
-
- $XFS_REPAIR_PROG -n $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
- if [ $? -ne 0 ]; then
- echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild-reverify) (see $seqres.full)"
-
- echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild-reverify)" >>$seqres.full
- echo "*** xfs_repair -n output ***" >>$seqres.full
- cat $tmp.repair | _fix_malloc >>$seqres.full
- echo "*** end xfs_repair output" >>$seqres.full
-
- ok=0
- fi
- rm -f $tmp.repair
- fi
-
- if [ $ok -eq 0 ]
- then
- echo "*** mount output ***" >>$seqres.full
- _mount >>$seqres.full
- echo "*** end mount output" >>$seqres.full
- elif [ "$type" = "xfs" ]
- then
- _mount_or_remount_rw "$extra_mount_options" $device $mountpoint
- fi
-
- if [ $ok -eq 0 ]; then
- status=1
- if [ "$iam" != "check" ]; then
+ if [ $# -ne 3 ]; then
+ echo "Usage: _check_xfs_filesystem device <logdev>|none <rtdev>|none" 1>&2
exit 1
fi
- return 1
- fi
- return 0
+ extra_mount_options=""
+ extra_log_options=""
+ extra_options=""
+ device=$1
+ if [ -f $device ]; then
+ extra_options="-f"
+ fi
+
+ if [ "$2" != "none" ]; then
+ extra_log_options="-l$2"
+ extra_mount_options="-ologdev=$2"
+ fi
+
+ if [ "$3" != "none" ]; then
+ extra_rt_options="-r$3"
+ extra_mount_options=$extra_mount_options" -ortdev=$3"
+ fi
+ extra_mount_options=$extra_mount_options" $MOUNT_OPTIONS"
+
+ [ "$FSTYP" != xfs ] && return 0
+
+ type=`_fs_type $device`
+ ok=1
+
+ if [ "$type" = "xfs" ]; then
+ if [ -n "$TEST_XFS_SCRUB" ] && [ -x "$XFS_SCRUB_PROG" ]; then
+ "$XFS_SCRUB_PROG" $scrubflag -vd $device >>$seqres.full
+ if [ $? -ne 0 ]; then
+ echo "filesystem on $device failed scrub (see $seqres.full)"
+ ok=0
+ fi
+ fi
+ # mounted ...
+ mountpoint=`_umount_or_remount_ro $device`
+ fi
+
+ $XFS_LOGPRINT_PROG -t $extra_log_options $device 2>&1 \
+ | tee $tmp.logprint | grep -q "<CLEAN>"
+ if [ $? -ne 0 -a "$HOSTOS" = "Linux" ]; then
+ echo "_check_xfs_filesystem: filesystem on $device has dirty log (see $seqres.full)"
+
+ echo "_check_xfs_filesystem: filesystem on $device has dirty log" >>$seqres.full
+ echo "*** xfs_logprint -t output ***" >>$seqres.full
+ cat $tmp.logprint >>$seqres.full
+ echo "*** end xfs_logprint output" >>$seqres.full
+
+ ok=0
+ fi
+
+ # xfs_check runs out of memory on large files, so even providing the test
+ # option (-t) to avoid indexing the free space trees doesn't make it pass on
+ # large filesystems. Avoid it.
+ if [ "$LARGE_SCRATCH_DEV" != yes ]; then
+ _xfs_check $extra_log_options $device 2>&1 |\
+ _fix_malloc >$tmp.fs_check
+ fi
+ if [ -s $tmp.fs_check ]; then
+ echo "_check_xfs_filesystem: filesystem on $device is inconsistent (c) (see $seqres.full)"
+
+ echo "_check_xfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
+ echo "*** xfs_check output ***" >>$seqres.full
+ cat $tmp.fs_check >>$seqres.full
+ echo "*** end xfs_check output" >>$seqres.full
+
+ ok=0
+ fi
+
+ $XFS_REPAIR_PROG -n $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
+ if [ $? -ne 0 ]; then
+ echo "_check_xfs_filesystem: filesystem on $device is inconsistent (r) (see $seqres.full)"
+
+ echo "_check_xfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
+ echo "*** xfs_repair -n output ***" >>$seqres.full
+ cat $tmp.repair | _fix_malloc >>$seqres.full
+ echo "*** end xfs_repair output" >>$seqres.full
+
+ ok=0
+ fi
+ rm -f $tmp.fs_check $tmp.logprint $tmp.repair
+
+ # Optionally test the index rebuilding behavior.
+ if [ -n "$TEST_XFS_REPAIR_REBUILD" ]; then
+ $XFS_REPAIR_PROG $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
+ if [ $? -ne 0 ]; then
+ echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild) (see $seqres.full)"
+
+ echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild)" >>$seqres.full
+ echo "*** xfs_repair output ***" >>$seqres.full
+ cat $tmp.repair | _fix_malloc >>$seqres.full
+ echo "*** end xfs_repair output" >>$seqres.full
+
+ ok=0
+ fi
+ rm -f $tmp.repair
+
+ $XFS_REPAIR_PROG -n $extra_options $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
+ if [ $? -ne 0 ]; then
+ echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild-reverify) (see $seqres.full)"
+
+ echo "_check_xfs_filesystem: filesystem on $device is inconsistent (rebuild-reverify)" >>$seqres.full
+ echo "*** xfs_repair -n output ***" >>$seqres.full
+ cat $tmp.repair | _fix_malloc >>$seqres.full
+ echo "*** end xfs_repair output" >>$seqres.full
+
+ ok=0
+ fi
+ rm -f $tmp.repair
+ fi
+
+ if [ $ok -eq 0 ]; then
+ echo "*** mount output ***" >>$seqres.full
+ _mount >>$seqres.full
+ echo "*** end mount output" >>$seqres.full
+ elif [ "$type" = "xfs" ]; then
+ _mount_or_remount_rw "$extra_mount_options" $device $mountpoint
+ fi
+
+ if [ $ok -eq 0 ]; then
+ status=1
+ if [ "$iam" != "check" ]; then
+ exit 1
+ fi
+ return 1
+ fi
+
+ return 0
}
_check_xfs_test_fs()
{
- TEST_LOG="none"
- TEST_RT="none"
- [ "$USE_EXTERNAL" = yes -a ! -z "$TEST_LOGDEV" ] && \
- TEST_LOG="$TEST_LOGDEV"
-
- [ "$USE_EXTERNAL" = yes -a ! -z "$TEST_RTDEV" ] && \
- TEST_RT="$TEST_RTDEV"
-
- _check_xfs_filesystem $TEST_DEV $TEST_LOG $TEST_RT
-
- # check for ipath consistency
- if $XFS_GROWFS_PROG -n $TEST_DIR | grep -q 'inode-paths=1'; then
- # errors go to stderr
- xfs_check_ipaths $TEST_DIR >/dev/null
- xfs_repair_ipaths -n $TEST_DIR >/dev/null
- fi
+ TEST_LOG="none"
+ TEST_RT="none"
+ [ "$USE_EXTERNAL" = yes -a ! -z "$TEST_LOGDEV" ] && \
+ TEST_LOG="$TEST_LOGDEV"
+
+ [ "$USE_EXTERNAL" = yes -a ! -z "$TEST_RTDEV" ] && \
+ TEST_RT="$TEST_RTDEV"
+
+ _check_xfs_filesystem $TEST_DEV $TEST_LOG $TEST_RT
+
+ # check for ipath consistency
+ if $XFS_GROWFS_PROG -n $TEST_DIR | grep -q 'inode-paths=1'; then
+ # errors go to stderr
+ xfs_check_ipaths $TEST_DIR >/dev/null
+ xfs_repair_ipaths -n $TEST_DIR >/dev/null
+ fi
}
_require_xfs_test_rmapbt()
@@ -505,7 +496,8 @@ _require_xfs_scratch_rmapbt()
_scratch_unmount
}
-_xfs_bmapx_find() {
+_xfs_bmapx_find()
+{
case "$1" in
"attr")
param="a"
@@ -535,7 +527,7 @@ _reset_xfs_sysfs_error_handling()
{
local dev=$1
- if [ ! -b "$dev" -o "$FSTYP" != "xfs" ];then
+ if [ ! -b "$dev" -o "$FSTYP" != "xfs" ]; then
_fail "Usage: reset_xfs_sysfs_error_handling <device>"
fi
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] common: cleanups after common/rc split
2016-12-01 4:22 [PATCH] common: cleanups after common/rc split Eryu Guan
@ 2016-12-02 3:29 ` Dave Chinner
2016-12-02 3:35 ` Eryu Guan
0 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2016-12-02 3:29 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests
On Thu, Dec 01, 2016 at 12:22:13PM +0800, Eryu Guan wrote:
> Fix code style issues after the common/rc split, e.g. use tab
> indention, fix if-then-else format, fix while-do and for-do format
> and various whitespace issues. No functional changes.
Couple of quick things....
> + OPTS=" "
> + DBOPTS=" "
> + USAGE="Usage: xfs_check [-fsvV] [-l logdev] [-i ino]... [-b bno]... special"
> +
> + while getopts "b:fi:l:stvV" c; do
> + case $c in
> + s) OPTS=$OPTS"-s ";;
> + t) OPTS=$OPTS"-t ";;
> + v) OPTS=$OPTS"-v ";;
> + i) OPTS=$OPTS"-i "$OPTARG" ";;
> + b) OPTS=$OPTS"-b "$OPTARG" ";;
> + f) DBOPTS=$DBOPTS" -f";;
> + l) DBOPTS=$DBOPTS" -l "$OPTARG" ";;
> + V) $XFS_DB_PROG -p xfs_check -V
> + return $?
> + ;;
> + esac
It saves space and makes it easier to read if you don't indent the
individual cases but do indent the code so multi-line operations
don't end up with weird space indenting:
case $c in
s) OPTS=$OPTS"-s ";;
t) OPTS=$OPTS"-t ";;
v) OPTS=$OPTS"-v ";;
....
V) $XFS_DB_PROG -p xfs_check -V
return $?
;;
> @@ -198,23 +197,23 @@ _test_xfs_logprint()
>
> _scratch_xfs_check()
> {
> - SCRATCH_OPTIONS=""
> - [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> - SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
> - [ "$LARGE_SCRATCH_DEV" = yes ] && \
> - SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
> - _xfs_check $SCRATCH_OPTIONS $* $SCRATCH_DEV
> + SCRATCH_OPTIONS=""
> + [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> + SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
> + [ "$LARGE_SCRATCH_DEV" = yes ] && \
> + SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
If it's a multi-line construct, just use if [...]; then.
> + _xfs_check $SCRATCH_OPTIONS $* $SCRATCH_DEV
> }
>
> _scratch_xfs_repair()
> {
> - SCRATCH_OPTIONS=""
> - [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> - SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
> - [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
> - SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV"
> - [ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
> - $XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
> + SCRATCH_OPTIONS=""
> + [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> + SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
> + [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
> + SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV"
And this can become:
if [ "$USE_EXTERNAL" = yes ]; then
[ -n "$SCRATCH_LOGDEV" ] && SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
[ -n "$SCRATCH_RTDEV" ] && SCRATCH_OPTIONS="-l$SCRATCH_RTDEV"
fi
> +
> + $XFS_LOGPRINT_PROG -t $extra_log_options $device 2>&1 \
> + | tee $tmp.logprint | grep -q "<CLEAN>"
> + if [ $? -ne 0 -a "$HOSTOS" = "Linux" ]; then
> + echo "_check_xfs_filesystem: filesystem on $device has dirty log (see $seqres.full)"
> +
> + echo "_check_xfs_filesystem: filesystem on $device has dirty log" >>$seqres.full
> + echo "*** xfs_logprint -t output ***" >>$seqres.full
> + cat $tmp.logprint >>$seqres.full
> + echo "*** end xfs_logprint output" >>$seqres.full
Better, I think, with these multi-line file dumps is something like:
(
echo ....
echo ....
cat ...
echo
) >> $seqres.full
> + if [ $ok -eq 0 ]; then
> + echo "*** mount output ***" >>$seqres.full
> + _mount >>$seqres.full
> + echo "*** end mount output" >>$seqres.full
> + elif [ "$type" = "xfs" ]; then
> + _mount_or_remount_rw "$extra_mount_options" $device $mountpoint
> + fi
> +
> + if [ $ok -eq 0 ]; then
> + status=1
> + if [ "$iam" != "check" ]; then
> + exit 1
> + fi
> + return 1
> + fi
THese $ok -eq 0 cases can be combined.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] common: cleanups after common/rc split
2016-12-02 3:29 ` Dave Chinner
@ 2016-12-02 3:35 ` Eryu Guan
2016-12-02 6:25 ` Eryu Guan
0 siblings, 1 reply; 5+ messages in thread
From: Eryu Guan @ 2016-12-02 3:35 UTC (permalink / raw)
To: Dave Chinner; +Cc: fstests
On Fri, Dec 02, 2016 at 02:29:35PM +1100, Dave Chinner wrote:
> On Thu, Dec 01, 2016 at 12:22:13PM +0800, Eryu Guan wrote:
> > Fix code style issues after the common/rc split, e.g. use tab
> > indention, fix if-then-else format, fix while-do and for-do format
> > and various whitespace issues. No functional changes.
>
> Couple of quick things....
>
> > + OPTS=" "
> > + DBOPTS=" "
> > + USAGE="Usage: xfs_check [-fsvV] [-l logdev] [-i ino]... [-b bno]... special"
> > +
> > + while getopts "b:fi:l:stvV" c; do
> > + case $c in
> > + s) OPTS=$OPTS"-s ";;
> > + t) OPTS=$OPTS"-t ";;
> > + v) OPTS=$OPTS"-v ";;
> > + i) OPTS=$OPTS"-i "$OPTARG" ";;
> > + b) OPTS=$OPTS"-b "$OPTARG" ";;
> > + f) DBOPTS=$DBOPTS" -f";;
> > + l) DBOPTS=$DBOPTS" -l "$OPTARG" ";;
> > + V) $XFS_DB_PROG -p xfs_check -V
> > + return $?
> > + ;;
> > + esac
>
> It saves space and makes it easier to read if you don't indent the
> individual cases but do indent the code so multi-line operations
> don't end up with weird space indenting:
>
> case $c in
> s) OPTS=$OPTS"-s ";;
> t) OPTS=$OPTS"-t ";;
> v) OPTS=$OPTS"-v ";;
> ....
> V) $XFS_DB_PROG -p xfs_check -V
> return $?
> ;;
>
Yes, this looks better.
> > @@ -198,23 +197,23 @@ _test_xfs_logprint()
> >
> > _scratch_xfs_check()
> > {
> > - SCRATCH_OPTIONS=""
> > - [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> > - SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
> > - [ "$LARGE_SCRATCH_DEV" = yes ] && \
> > - SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
> > - _xfs_check $SCRATCH_OPTIONS $* $SCRATCH_DEV
> > + SCRATCH_OPTIONS=""
> > + [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> > + SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
> > + [ "$LARGE_SCRATCH_DEV" = yes ] && \
> > + SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
>
> If it's a multi-line construct, just use if [...]; then.
I didn't pay attention to the actual code, just fixed the indention
issue. I'll fix them in v2.
Thanks for the review!
Eryu
>
> > + _xfs_check $SCRATCH_OPTIONS $* $SCRATCH_DEV
> > }
> >
> > _scratch_xfs_repair()
> > {
> > - SCRATCH_OPTIONS=""
> > - [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> > - SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
> > - [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
> > - SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV"
> > - [ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
> > - $XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
> > + SCRATCH_OPTIONS=""
> > + [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> > + SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
> > + [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
> > + SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV"
>
> And this can become:
>
> if [ "$USE_EXTERNAL" = yes ]; then
> [ -n "$SCRATCH_LOGDEV" ] && SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
> [ -n "$SCRATCH_RTDEV" ] && SCRATCH_OPTIONS="-l$SCRATCH_RTDEV"
> fi
>
> > +
> > + $XFS_LOGPRINT_PROG -t $extra_log_options $device 2>&1 \
> > + | tee $tmp.logprint | grep -q "<CLEAN>"
> > + if [ $? -ne 0 -a "$HOSTOS" = "Linux" ]; then
> > + echo "_check_xfs_filesystem: filesystem on $device has dirty log (see $seqres.full)"
> > +
> > + echo "_check_xfs_filesystem: filesystem on $device has dirty log" >>$seqres.full
> > + echo "*** xfs_logprint -t output ***" >>$seqres.full
> > + cat $tmp.logprint >>$seqres.full
> > + echo "*** end xfs_logprint output" >>$seqres.full
>
> Better, I think, with these multi-line file dumps is something like:
>
> (
> echo ....
> echo ....
> cat ...
> echo
> ) >> $seqres.full
> > + if [ $ok -eq 0 ]; then
> > + echo "*** mount output ***" >>$seqres.full
> > + _mount >>$seqres.full
> > + echo "*** end mount output" >>$seqres.full
> > + elif [ "$type" = "xfs" ]; then
> > + _mount_or_remount_rw "$extra_mount_options" $device $mountpoint
> > + fi
> > +
> > + if [ $ok -eq 0 ]; then
> > + status=1
> > + if [ "$iam" != "check" ]; then
> > + exit 1
> > + fi
> > + return 1
> > + fi
>
> THese $ok -eq 0 cases can be combined.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] common: cleanups after common/rc split
2016-12-02 3:35 ` Eryu Guan
@ 2016-12-02 6:25 ` Eryu Guan
2016-12-02 6:51 ` Dave Chinner
0 siblings, 1 reply; 5+ messages in thread
From: Eryu Guan @ 2016-12-02 6:25 UTC (permalink / raw)
To: Dave Chinner; +Cc: fstests
On Fri, Dec 02, 2016 at 11:35:04AM +0800, Eryu Guan wrote:
...
> > > + if [ $ok -eq 0 ]; then
> > > + echo "*** mount output ***" >>$seqres.full
> > > + _mount >>$seqres.full
> > > + echo "*** end mount output" >>$seqres.full
> > > + elif [ "$type" = "xfs" ]; then
> > > + _mount_or_remount_rw "$extra_mount_options" $device $mountpoint
I looked at this more and I think that a "ok=$?" is missed here
(_mount_or_remount_rw return 1 as success), both
_check_generic_filesystem and _check_btrfs_filesystem do this. So if it
passes the first $ok check then does this rw mount and sets $ok based on
the mount result. I'll just add a "ok=$?" here.
Thanks,
Eryu
> > > + fi
> > > +
> > > + if [ $ok -eq 0 ]; then
> > > + status=1
> > > + if [ "$iam" != "check" ]; then
> > > + exit 1
> > > + fi
> > > + return 1
> > > + fi
> >
> > THese $ok -eq 0 cases can be combined.
> >
> > Cheers,
> >
> > Dave.
> > --
> > Dave Chinner
> > david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] common: cleanups after common/rc split
2016-12-02 6:25 ` Eryu Guan
@ 2016-12-02 6:51 ` Dave Chinner
0 siblings, 0 replies; 5+ messages in thread
From: Dave Chinner @ 2016-12-02 6:51 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests
On Fri, Dec 02, 2016 at 02:25:33PM +0800, Eryu Guan wrote:
> On Fri, Dec 02, 2016 at 11:35:04AM +0800, Eryu Guan wrote:
> ...
> > > > + if [ $ok -eq 0 ]; then
> > > > + echo "*** mount output ***" >>$seqres.full
> > > > + _mount >>$seqres.full
> > > > + echo "*** end mount output" >>$seqres.full
> > > > + elif [ "$type" = "xfs" ]; then
> > > > + _mount_or_remount_rw "$extra_mount_options" $device $mountpoint
>
> I looked at this more and I think that a "ok=$?" is missed here
> (_mount_or_remount_rw return 1 as success), both
> _check_generic_filesystem and _check_btrfs_filesystem do this. So if it
> passes the first $ok check then does this rw mount and sets $ok based on
> the mount result. I'll just add a "ok=$?" here.
Sounds good. :P
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-02 6:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-01 4:22 [PATCH] common: cleanups after common/rc split Eryu Guan
2016-12-02 3:29 ` Dave Chinner
2016-12-02 3:35 ` Eryu Guan
2016-12-02 6:25 ` Eryu Guan
2016-12-02 6:51 ` Dave Chinner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox