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@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
