Created SC2310 (markdown)

Vidar Holen
2021-09-04 17:00:12 -07:00
parent 5999174bfc
commit 5deb2a8d60

57
SC2310.md Normal file

@@ -0,0 +1,57 @@
## This function is invoked in an 'if' condition so set -e will be disabled. Invoke separately if failures should cause the script to exit.
(This warning is optional and must be explicitly enabled)
### Problematic code:
```sh
#!/bin/sh
#shellcheck enable=check-set-e-suppressed
set -e
backup() {
cp *.txt /backup
rm *.txt # Runs even if copy fails!
}
if backup
then
echo "Backup successful"
fi
```
### Correct code:
```sh
#!/bin/sh
#shellcheck enable=check-set-e-suppressed
set -e
backup() {
cp *.txt /backup
rm *.txt
}
backup
echo "Backup successful"
```
### Rationale:
ShellCheck found a function used as a condition in a script where `set -e` is enabled. This means that the function will run without `set -e`, and will power through any errors.
This applies to `if`, `while`, and `until` statements, commands negated with `!`, as well as the left-hand side of `||` and `&&`. It does matter how deeply the command is nested in such a structure.
In the problematic example, the intent was that an error like `cp: error writing '/backup/important.txt': No space left on device` would cause the script to abort. Instead, since the function is invoked in an `if` statement, the script will proceed to delete all the files even though it failed to back them up.
The fix is to call it outside of an `if` statement. There is no point in checking whether the command succeeded, since the script would abort if it didn't. You may also want to consider replacing `set -e` with explicit `|| exit` after every relevant command to avoid such surprises.
### Exceptions:
If you don't care that the function runs without `set -e`, you can disable this warning.
### Related resources:
* BashFaq #105: [Why doesn't set -e (or set -o errexit, or trap ERR) do what I expected?](https://mywiki.wooledge.org/BashFAQ/105)