#1542 PR merged: Some code simplification plus explicit 'return 0' where needed

Labels: documentation, cleanup, fixed / solved / done, critical / security / legal

jsmeix opened issue at 2017-10-24 09:13:

Some simplification in code that is related to issue 1541
plus explicit 'return 0' when return is intended after
a failed command i.e. when return is actually 'success'
because otherwise return results the exit code of the
failed command e.g. as in

# function testfunc1 () { test -f qqq || return ; }
# testfunc1
# echo $?

# function testfunc2 () { grep -Q foo bar && echo found ; return ; }
# testfunc2
grep: invalid option -- 'Q'
Usage: grep [OPTION]... PATTERN [FILE]...
Try `grep --help' for more information.
# echo $?

jsmeix commented at 2017-10-24 09:20:

in usr/share/rear/prep/GNU/Linux/300_include_grub_tools.sh
I added a 'FIXME' for you because via
you added

grubdir=$(ls -d /boot/grub*)

which is not fail-safe because
usr/sbin/rear sets 'shopt -s nullglob'
so that the 'ls' command will list all files in the current
working directory if nothing matches the bash globbing
pattern '/boot/grub*' which results in this case '.' in 'grubdir'
(the plain 'ls -d' output in the current working directory).

jsmeix commented at 2017-10-24 10:29:

do you have a good idea how one could make things like

grubdir=$( ls -d /boot/grub* )

work fail-safe even with 'shopt -s nullglob'?

I would like to get the 'FIXME' fixed.

What I did in such cases was to not use 'ls' but 'find'
e.g. as in prep/NETFS/default/070_set_backup_archive.sh

# Here things like 'find /path/to/dir -name '*.tar.gz' | sort' are used because
# one cannot use bash globbing via commands like 'ls /path/to/dir/*.tar.gz'
# because /usr/sbin/rear sets the nullglob bash option which leads to plain 'ls'
# when '/path/to/dir/*.tar.gz' matches nothing (i.e. when no backup file exists)
# so that then plain 'ls' would result nonsense.
local latest_full_backup=$( find $backup_directory -name "$full_backup_glob_regex" | sort | tail -n1 )

jsmeix commented at 2017-10-24 15:07:

I think I found another possible way
how to be fail-safe for 'shopt -s nullglob':

Do not use

ls globbing_pattern

but (where possible)

echo -n globbing_pattern

because with 'shopt -s nullglob' the 'echo -n' command
outputs nothing if nothing matches the globbing pattern
so that one can test for an empty variable like:

local grubdir="$( echo -n /boot/grub* )"
# Set fallback if nothing matches '/boot/grub*'
test -d "$grubdir" || grubdir='/boot/grub'

I implemented that in

gdha commented at 2017-10-24 15:25:

@jsmeix excellent work - thank you for that

jsmeix commented at 2017-10-25 10:31:

In a ReaR git checkout directory I did things like

# find usr/sbin/rear usr/share/rear/ | xargs grep '=$(ls '

# find usr/sbin/rear usr/share/rear/ | xargs grep '=$( ls '

# find usr/sbin/rear usr/share/rear/ | xargs grep '="$(ls '

# find usr/sbin/rear usr/share/rear/ | xargs grep '="$( ls '

and found

var=$( ls globbing_pattern )

usage that needs to be fixed only in

Other such usage in
looks to be already prepared for 'shopt -s nullglob'
so that I only added an explanatory comment there.


jsmeix commented at 2017-10-25 14:39:

Because all still works for me I "just merge" it, cf.

[Export of Github issue for rear/rear.]