How to Do a Code Review of Bash Scripts

Conducting a code review for Bash scripts is essential to ensure they are error-free, secure, and easy to maintain. Reviewing Bash scripts helps catch mistakes early, improve code quality, and ensures best practices are followed. Here's a detailed guide on how to review Bash scripts effectively, with explanations and examples of good and bad code for each step.

1. Understand the Purpose of the Script

Before reviewing, understand what the script is supposed to do. This helps in contextualizing the code and spotting deviations.

Good:

# This script backs up the user's home directory to /backup

Bad:

# backup script

2. Check for Shebang and Execution Permissions

Ensure the script starts with a shebang to specify the interpreter and that it has executable permissions.

Good:

#!/bin/bash
chmod +x script.sh

Bad:

#!/bin/sh

3. Syntax and Semantics

Look for syntax errors and semantic issues. Use tools like shellcheck to detect common mistakes.

Good:

if [ -f "$file" ]; then
    echo "File exists."
fi

Bad:

if [ -f "$file" ] then
    echo "File exists."

4. Readability and Maintainability

Check for proper indentation, meaningful variable names, and adequate comments.

Good:

for file in *.txt; do
    echo "Processing $file"
done

Bad:

for f in *.txt; do echo "Processing $f"; done

5. Error Handling

Ensure the script handles errors gracefully using proper error handling mechanisms.

Good:

set -euo pipefail
trap 'echo "Error occurred"; exit 1' ERR

Bad:

# No error handling

6. Security Considerations

Look for potential security issues like unchecked user input and improper handling of sensitive data.

Good:

if [[ "$user_input" =~ ^[a-zA-Z0-9_]+$ ]]; then
    echo "Valid input"
fi

Bad:

eval $user_input

7. Performance and Efficiency

Assess the script for performance bottlenecks and unnecessary use of resources.

Good:

grep "pattern" file.txt

Bad:

cat file.txt | grep "pattern"

8. Adherence to Best Practices

Ensure the script follows best practices for Bash scripting.

Good:

result=$(command)

Bad:

result=`command`

9. Dependency Management

Identify any external dependencies and ensure they are clearly documented.

Good:

# Requires rsync
if ! command -v rsync &> /dev/null; then
    echo "rsync could not be found"
    exit 1
fi

Bad:

rsync -avh source/ destination/

10. Portability

Check if the script uses features or commands specific to a particular shell or system.

Good:

# POSIX compliant
if [ -d "$DIR" ]; then
    echo "Directory exists."
fi

Bad:

[[ -d "$DIR" ]] && echo "Directory exists."

11. Documentation

Verify that the script includes a header comment explaining its purpose and usage instructions.

Good:

# Script to backup user's home directory
# Usage: ./backup.sh

Bad:

# Backup script

12. Testing

Ensure the script has been tested in different environments and scenarios.

Good:

# Test script
./test_backup.sh

Bad:

# No testing

13. Variable Naming

Use meaningful and descriptive variable names to improve readability.

Good:

file_count=0

Bad:

fc=0

14. Avoid Hardcoding Values

Use variables instead of hardcoding values to make the script more flexible.

Good:

backup_dir="/backup"

Bad:

cd /backup

15. Use Functions for Reusable Code

Encapsulate reusable code in functions to improve modularity and readability.

Good:

backup_files() {
    tar -czf backup.tar.gz /home/user
}

Bad:

tar -czf backup.tar.gz /home/user

16. Check Command Success

Always check if a command succeeded and handle the failure case appropriately.

Good:

if ! cp source.txt destination.txt; then
    echo "Copy failed"
    exit 1
fi

Bad:

cp source.txt destination.txt

17. Use Meaningful Exit Codes

Use appropriate exit codes to indicate the script's status.

Good:

exit 0

Bad:

exit 1

18. Avoid Useless Use of cat

Combine commands to avoid unnecessary use of cat.

Good:

grep "pattern" file.txt

Bad:

cat file.txt | grep "pattern"

19. Quotes Around Variables

Always quote variables to prevent word splitting and globbing issues.

Good:

echo "File: $file"

Bad:

echo File: $file

20. Avoid Global Variables

Use local variables within functions to avoid side effects.

Good:

main() {
    local file_count=0
}

Bad:

file_count=0

21. Proper Use of Arrays

Use arrays for lists of items to simplify the code.

Good:

files=(file1.txt file2.txt)
for file in "${files[@]}"; do
    echo "Processing $file"
done

Bad:

file1=file1.txt
file2=file2.txt
for file in $file1 $file2; do
    echo "Processing $file"
done

22. Avoiding Command Substitution in Loops

Avoid using command substitution within loops for better performance.

Good:

while read -r line; do
    echo "$line"
done < file.txt

Bad:

for line in $(cat file.txt); do
    echo "$line"
done

23. Proper Use of printf

Use printf instead of echo for better formatting control.

Good:

printf "File: %s\n" "$file"

Bad:

echo "File: $file"

24. Check for Unset Variables

Use set -u to treat unset variables as an error.

Good:

set -u
echo "Variable: ${var:-default}"

Bad:

echo "Variable: $var"

25. Proper Use of trap

Use trap to handle cleanup tasks and ensure they run even if the script exits unexpectedly.

Good:

trap 'rm -f temp.txt; exit' INT TERM

Bad:

# No cleanup

26. Avoiding Multiple Redirections

Combine redirections to avoid multiple file handles.

Good:

{
    echo "Line 1"
    echo "Line 2"
} > output.txt

Bad:

echo "Line 1" > output.txt
echo "Line 2" >> output.txt

27. Using Built-in Shell Commands

Prefer built-in shell commands over external utilities where possible.

Good:

files=$(ls)

Bad:

files=$(ls -1)

28. Avoiding the Use of eval

Avoid eval to prevent potential security risks.

Good:

cmd="ls"
$cmd

Bad:

eval $cmd

29. Proper Use of read

Use read with proper options to handle input safely.

Good:

read -r user_input

Bad:

read user_input

30. Using || and && for Command Chaining

Use || and && for conditional command execution.

Good:

command1 && command2
command1 || echo "Command1 failed"

Bad:

if command1; then
    command2
fi
if ! command1; then
    echo "Command1 failed"
fi

31. Using case Instead of Multiple if Statements

Use case for multiple conditions to improve readability.

Good:

case $var in
    pattern1) echo "Pattern 1";;
    pattern2) echo "Pattern 

2";;
esac

Bad:

if [ "$var" == "pattern1" ]; then
    echo "Pattern 1"
elif [ "$var" == "pattern2" ]; then
    echo "Pattern 2"
fi

32. Properly Handling File Descriptors

Use file descriptors to manage input/output streams efficiently.

Good:

exec 3< input.txt
while read -r line <&3; do
    echo "$line"
done
exec 3<&-

Bad:

while read -r line; do
    echo "$line"
done < input.txt

33. Using select for Menu Options

Use select to create simple menus.

Good:

select option in "Option 1" "Option 2" "Quit"; do
    case $option in
        "Option 1") echo "You chose Option 1";;
        "Option 2") echo "You chose Option 2";;
        "Quit") break;;
    esac
done

Bad:

echo "1. Option 1"
echo "2. Option 2"
echo "3. Quit"
read -r choice
case $choice in
    1) echo "You chose Option 1";;
    2) echo "You chose Option 2";;
    3) exit;;
esac

34. Using dirname and basename

Use dirname and basename to handle file paths.

Good:

dir=$(dirname "$file_path")
file=$(basename "$file_path")

Bad:

dir=${file_path%/*}
file=${file_path##*/}

35. Using mktemp for Temporary Files

Use mktemp to create temporary files securely.

Good:

tmpfile=$(mktemp)
echo "Temporary file: $tmpfile"

Bad:

tmpfile="/tmp/tempfile.$$"
echo "Temporary file: $tmpfile"

By following these guidelines and using these examples, you can conduct a thorough and effective code review of Bash scripts, ensuring they are robust, secure, and maintainable.

For more advanced Bash scripting tips, check out this article on Advanced String Operations in Bash: Building Custom Functions.