| Message ID | 1425900805-11010-1-git-send-email-dsterba@suse.cz (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
-------- Original Message -------- Subject: [PATCH] btrfs-progs: tests, clean up scripts From: David Sterba <dsterba@suse.cz> To: <linux-btrfs@vger.kernel.org> Date: 2015?03?09? 19:33 > Rename variables, use caps, call true by full path, add quotation to > variables and a few wording fixes. > > Signed-off-by: David Sterba <dsterba@suse.cz> Much better than my original one except one enhancement and one small bug. > --- > tests/common | 43 ++++++++++++++-------------- > tests/fsck-tests/012-leaf-corruption/test.sh | 10 +++---- > 2 files changed, 27 insertions(+), 26 deletions(-) > > diff --git a/tests/common b/tests/common > index ccdadd5f248e..a35d438efe8f 100644 > --- a/tests/common > +++ b/tests/common > @@ -57,26 +57,26 @@ check_all_images() > # some tests need to mount the recovered image and do verifications call > # 'setup_root_helper' and then check for have_root_helper == 1 if the test > # needs to fail otherwise; using sudo by default for now > -_sudo= > -need_validate=-1 > -export _sudo > -export need_validate > +SUDO_HELPER= > +NEED_SUDO_VALIDATE=unknown Better quoted? > +export SUDO_HELPER > +export NEED_SUDO_VALIDATE > root_helper() > { > if [ $UID -eq 0 ]; then > - $* > + "$@" > else > - if [ $need_validate -eq 1 ]; then > - sudo -v -n &> /dev/null || \ > - _not_run "Need validate sudo credential" > - sudo -n $* > - elif [ $need_validate -eq 0 ]; then > - sudo -n true &> /dev/null || \ > - _not_run "Need validate sudo user setting" > - sudo -n $* > + if [ "$NEED_SUDO_VALIDATE" = 'yes' ]; then > + sudo -v -n &>/dev/null || \ > + _not_run "Need to validate sudo credentials" > + sudo -n "$@" > + elif [ "$NEED_SUDO_VALIDATE" = 'no' ]; then > + sudo -n /bin/true &> /dev/null || \ > + _not_run "Need to validate sudo user settings" > + sudo -n "$@" > else > # should not happen > - _not_run "Need validate root privilege" > + _not_run "Need to validate root privileges" > fi > fi > } > @@ -86,15 +86,16 @@ setup_root_helper() > if [ $UID -eq 0 ]; then > return > fi > - # Test for old sudo or special setting, which makes sudo -v fails even > - # user is set NOPASSWD > - sudo -n true &> /dev/null && need_validate=0 > + > + # Test for old sudo or special settings, which make sudo -v fail even > + # if user setting is NOPASSWD > + sudo -n /bin/true &>/dev/null && NEED_SUDO_VALIDATE=no > > # Newer sudo or default sudo setting > - sudo -v -n &> /dev/null && need_validate=1 > + sudo -v -n &>/dev/null && NEED_SUDO_VALIDATE=yes > > - if [ $need_validate -eq -1 ]; then > - _not_run "Need validate root privilege" > + if [ "$NEED_SUDO_VALIDATE" = 'yes' ]; then Shouldn't it be "$NEED_SUDO_VALIDATE" = 'unknown'? Thanks, Qu > + _not_run "Need to validate root privileges" > fi > - _sudo=root_helper > + SUDO_HELPER=root_helper > } > diff --git a/tests/fsck-tests/012-leaf-corruption/test.sh b/tests/fsck-tests/012-leaf-corruption/test.sh > index 5873e3fb42e3..8f82fd164d1d 100755 > --- a/tests/fsck-tests/012-leaf-corruption/test.sh > +++ b/tests/fsck-tests/012-leaf-corruption/test.sh > @@ -55,20 +55,20 @@ check_inode() > name=$5 > > # Check whether the inode exists > - exists=$($_sudo find $path -inum $ino) > + exists=$($SUDO_HELPER find $path -inum $ino) > if [ -z "$exists" ]; then > _fail "inode $ino not recovered correctly" > fi > > # Check inode type > - found_mode=$(printf "%o" 0x$($_sudo stat $exists -c %f)) > + found_mode=$(printf "%o" 0x$($SUDO_HELPER stat $exists -c %f)) > if [ $found_mode -ne $mode ]; then > echo "$found_mode" > _fail "inode $ino modes not recovered" > fi > > # Check inode size > - found_size=$($_sudo stat $exists -c %s) > + found_size=$($SUDO_HELPER stat $exists -c %s) > if [ $mode -ne 41700 -a $found_size -ne $size ]; then > _fail "inode $ino size not recovered correctly" > fi > @@ -90,7 +90,7 @@ check_leaf_corrupt_no_data_ext() > TEST_MNT="$(pwd)/tmp" > fi > mkdir -p $TEST_MNT || _fail "failed to create mount point" > - $_sudo mount $image -o ro $TEST_MNT > + $SUDO_HELPER mount $image -o ro $TEST_MNT > > i=0 > while [ $i -lt ${#leaf_no_data_ext_list[@]} ]; do > @@ -102,7 +102,7 @@ check_leaf_corrupt_no_data_ext() > ${leaf_no_data_ext_list[i + 4]} > ((i+=4)) > done > - $_sudo umount $TEST_MNT > + $SUDO_HELPER umount $TEST_MNT > } > > setup_root_helper > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 10, 2015 at 09:07:06AM +0800, Qu Wenruo wrote: > > +SUDO_HELPER= > > +NEED_SUDO_VALIDATE=unknown > Better quoted? Not needed here, no sideefects from evaluation and no special chars. > > @@ -86,15 +86,16 @@ setup_root_helper() > > if [ $UID -eq 0 ]; then > > return > > fi > > - # Test for old sudo or special setting, which makes sudo -v fails even > > - # user is set NOPASSWD > > - sudo -n true &> /dev/null && need_validate=0 > > + > > + # Test for old sudo or special settings, which make sudo -v fail even > > + # if user setting is NOPASSWD > > + sudo -n /bin/true &>/dev/null && NEED_SUDO_VALIDATE=no > > > > # Newer sudo or default sudo setting > > - sudo -v -n &> /dev/null && need_validate=1 > > + sudo -v -n &>/dev/null && NEED_SUDO_VALIDATE=yes > > > > - if [ $need_validate -eq -1 ]; then > > - _not_run "Need validate root privilege" > > + if [ "$NEED_SUDO_VALIDATE" = 'yes' ]; then > Shouldn't it be "$NEED_SUDO_VALIDATE" = 'unknown'? Right, thanks for catching it. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/common b/tests/common index ccdadd5f248e..a35d438efe8f 100644 --- a/tests/common +++ b/tests/common @@ -57,26 +57,26 @@ check_all_images() # some tests need to mount the recovered image and do verifications call # 'setup_root_helper' and then check for have_root_helper == 1 if the test # needs to fail otherwise; using sudo by default for now -_sudo= -need_validate=-1 -export _sudo -export need_validate +SUDO_HELPER= +NEED_SUDO_VALIDATE=unknown +export SUDO_HELPER +export NEED_SUDO_VALIDATE root_helper() { if [ $UID -eq 0 ]; then - $* + "$@" else - if [ $need_validate -eq 1 ]; then - sudo -v -n &> /dev/null || \ - _not_run "Need validate sudo credential" - sudo -n $* - elif [ $need_validate -eq 0 ]; then - sudo -n true &> /dev/null || \ - _not_run "Need validate sudo user setting" - sudo -n $* + if [ "$NEED_SUDO_VALIDATE" = 'yes' ]; then + sudo -v -n &>/dev/null || \ + _not_run "Need to validate sudo credentials" + sudo -n "$@" + elif [ "$NEED_SUDO_VALIDATE" = 'no' ]; then + sudo -n /bin/true &> /dev/null || \ + _not_run "Need to validate sudo user settings" + sudo -n "$@" else # should not happen - _not_run "Need validate root privilege" + _not_run "Need to validate root privileges" fi fi } @@ -86,15 +86,16 @@ setup_root_helper() if [ $UID -eq 0 ]; then return fi - # Test for old sudo or special setting, which makes sudo -v fails even - # user is set NOPASSWD - sudo -n true &> /dev/null && need_validate=0 + + # Test for old sudo or special settings, which make sudo -v fail even + # if user setting is NOPASSWD + sudo -n /bin/true &>/dev/null && NEED_SUDO_VALIDATE=no # Newer sudo or default sudo setting - sudo -v -n &> /dev/null && need_validate=1 + sudo -v -n &>/dev/null && NEED_SUDO_VALIDATE=yes - if [ $need_validate -eq -1 ]; then - _not_run "Need validate root privilege" + if [ "$NEED_SUDO_VALIDATE" = 'yes' ]; then + _not_run "Need to validate root privileges" fi - _sudo=root_helper + SUDO_HELPER=root_helper } diff --git a/tests/fsck-tests/012-leaf-corruption/test.sh b/tests/fsck-tests/012-leaf-corruption/test.sh index 5873e3fb42e3..8f82fd164d1d 100755 --- a/tests/fsck-tests/012-leaf-corruption/test.sh +++ b/tests/fsck-tests/012-leaf-corruption/test.sh @@ -55,20 +55,20 @@ check_inode() name=$5 # Check whether the inode exists - exists=$($_sudo find $path -inum $ino) + exists=$($SUDO_HELPER find $path -inum $ino) if [ -z "$exists" ]; then _fail "inode $ino not recovered correctly" fi # Check inode type - found_mode=$(printf "%o" 0x$($_sudo stat $exists -c %f)) + found_mode=$(printf "%o" 0x$($SUDO_HELPER stat $exists -c %f)) if [ $found_mode -ne $mode ]; then echo "$found_mode" _fail "inode $ino modes not recovered" fi # Check inode size - found_size=$($_sudo stat $exists -c %s) + found_size=$($SUDO_HELPER stat $exists -c %s) if [ $mode -ne 41700 -a $found_size -ne $size ]; then _fail "inode $ino size not recovered correctly" fi @@ -90,7 +90,7 @@ check_leaf_corrupt_no_data_ext() TEST_MNT="$(pwd)/tmp" fi mkdir -p $TEST_MNT || _fail "failed to create mount point" - $_sudo mount $image -o ro $TEST_MNT + $SUDO_HELPER mount $image -o ro $TEST_MNT i=0 while [ $i -lt ${#leaf_no_data_ext_list[@]} ]; do @@ -102,7 +102,7 @@ check_leaf_corrupt_no_data_ext() ${leaf_no_data_ext_list[i + 4]} ((i+=4)) done - $_sudo umount $TEST_MNT + $SUDO_HELPER umount $TEST_MNT } setup_root_helper
Rename variables, use caps, call true by full path, add quotation to variables and a few wording fixes. Signed-off-by: David Sterba <dsterba@suse.cz> --- tests/common | 43 ++++++++++++++-------------- tests/fsck-tests/012-leaf-corruption/test.sh | 10 +++---- 2 files changed, 27 insertions(+), 26 deletions(-)