librenms/librenms

View on GitHub
scripts/Migration/Standard_Conversion/convert_no_xml.sh

Summary

Maintainability
Test Coverage

Use ./*glob* or -- *glob* so names with dashes won't become options.
Open

        scp *.rrd root@$DEST:$L_RRDPATH"${line%/*}"/ 

Use ./*glob* or -- *glob* so names with dashes won't become options.

Problematic code:

rm *

Correct code:

rm ./*

or

rm -- *

Rationale

Since files and arguments are strings passed the same way, programs can't properly determine which is which, and rely on dashes to determine what's what.

A file named -f (touch -- -f) will not be deleted by the problematic code. It will instead be interpreted as a command line option, and rm will even report success.

Using ./* will instead cause the glob to be expanded into ./-f, which no program will treat as an option.

Similarly, -- by convention indicates the end of options, and nothing after it will be treated like flags (except for some programs possibly still special casing - as e.g. stdin).

Note that changing * to ./* in GNU Tar parameters will add ./ prefix to path names in the created archive. This may cause subtle problems (eg. to search for a specific file in archive, the ./ prefix must be specified as well). So using -- * is a safer fix for GNU Tar commands.

For more information, see "Filenames and Pathnames in Shell: How to do it Correctly".

Notice

Original content from the ShellCheck https://github.com/koalaman/shellcheck/wiki.

read without -r will mangle backslashes.
Open

while read line; 

read without -r mangle backslashes

Problematic code:

echo "Enter name:"
read name

Correct code:

echo "Enter name:"
read -r name

Rationale:

By default, read will interpret backslashes before spaces and line feeds, and otherwise strip them. This is rarely expected or desired.

Normally you just want to read data, which is what read -r does. You should always use -r unless you have a good reason not to.

Note that read -r will still strip leading and trailing spaces. IFS="" read -r prevents this.

Exceptions:

If you want backslashes to affect field splitting and line terminators instead of being read, you can disable this message with a [[directive]].

Notice

Original content from the ShellCheck https://github.com/koalaman/shellcheck/wiki.

Use a ( subshell ) to avoid having to cd back.
Open

        cd ..

Use a ( subshell ) to avoid having to cd back.

Problematic code:

for dir in */
do
  cd "$dir"
  convert index.png index.jpg
  cd ..
done

Correct code:

for dir in */
do
  (
  cd "$dir" || exit
  convert index.png index.jpg
  )
done

or

for dir in */
do
  cd "$dir" || exit
  convert index.png index.jpg
  cd ..
done

Rationale:

When doing cd dir; somestuff; cd .., cd dir can fail when permissions are lacking, if the dir was deleted, or if dir is actually a file.

In this case, somestuff will run in the wrong directory and cd .. will take you to an even more wrong directory. In a loop, this will likely cause the next cd to fail as well, propagating this error and running these commands far away from the intended directories.

Check cds exit status and/or use subshells to limit the effects of cd.

Exceptions

If you set variables you can't use a subshell. In that case, you should definitely check the exit status of cd, which will also silence this suggestion.

Notice

Original content from the ShellCheck https://github.com/koalaman/shellcheck/wiki.

Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
Open

    do cd $O_RRDPATH"${line%/*}" 

Use cd ... || exit in case cd fails.

Problematic code:

cd generated_files
rm -r *.c

func(){
  cd foo
  do_something
}

Correct code:

cd generated_files || exit
rm -r *.c

# For functions, you may want to use return:
func(){
  cd foo || return
  do_something
}

Rationale:

cd can fail for a variety of reasons: misspelled paths, missing directories, missing permissions, broken symlinks and more.

If/when it does, the script will keep going and do all its operations in the wrong directory. This can be messy, especially if the operations involve creating or deleting a lot of files.

To avoid this, make sure you handle the cases when cd fails. Ways to do this include

  • cd foo || exit as suggested to just abort immediately
  • if cd foo; then echo "Ok"; else echo "Fail"; fi for custom handling
  • <(cd foo && cmd) as an alternative to <(cd foo || exit; cmd) in <(..), $(..) or ( )

Exceptions:

ShellCheck does not give this warning when cd is on the left of a || or &&, or the condition of a if, while or until loop. Having a set -e command anywhere in the script will disable this message, even though it won't necessarily prevent the issue.

If you are accounting for cd failures in a way shellcheck doesn't realize, you can disable this message with a [[directive]].

Notice

Original content from the ShellCheck https://github.com/koalaman/shellcheck/wiki.

There are no issues that match your filters.

Category
Status