zigamilek zigamilek - 2 months ago 21
Linux Question

Optimizing bash script (for loops, permissions, ...)

I made this script (very quickly ... :)) ages ago and use it very often, but now I'm interested how bash experts would optimize it.

What it does is it goes through files and directories in the current directory and sets the correct permissions:

echo "Repairing chowns."
for item in "$@"; do
sudo chown -R ziga:ziga "$item"

echo "Setting chmods of directories to 755."
for item in $@; do
sudo find "$item" -type d -exec chmod 755 {} \;

echo "Setting chmods of files to 644."
for item in $@; do
sudo find "$item" -type f -exec chmod 644 {} \;

echo "Setting chmods of scripts to 744."
for item in $@; do
sudo find "$item" -type f -name "*.sh" -exec chmod 744 {} \;
sudo find "$item" -type f -name "*.pl" -exec chmod 744 {} \;
sudo find "$item" -type f -name "*.py" -exec chmod 744 {} \;

What I'd like to do is

  1. Reduce the number of for-loops

  2. Reduce the number of find statements (I know the last three can be combined into one, but I wonder if it can be reduced even further)

  3. Make script accept parameters other than the current directory and possibly accept multiple parameters (right now it only works if I cd into a desired directory and then call
    bash /home/ziga/scripts/repairPermissions.sh .
    ). NOTE: the parameters might have spaces in the path.


a) you are looping through $@ in all cases, you only need a single loop. a1) But find can do this for you, you don't need a bash loop. a2) And chown can take multiple directories as arguments.

b) Per chw21, remove the sudo for files you own.

c) One exec per found file/directory is expensive, use xargs.

d) Per chw21, combine the last three finds into one.

echo "Repairing permissions."
sudo chown -R ziga:ziga "$@"
find "$@" -type d -print0 | xargs -0 --no-run-if-empty chmod 755
find "$@" -type f -print0 | xargs -0 --no-run-if-empty chmod 644
find "$@" -type f \
   \( -name '*.sh' -o -name '*.pl' -o -name '*.py' \) \
   -print0 | xargs -0 --no-run-if-empty chmod 744

This is 11 execs (sudo, chown, 3 * find/xargs/chmod) of other processes (if the argument list is very long, xargs will exec chmod multiple times).

This does however read the directory tree four times. The OS's filesystem caching should help though.

Edit: Explanation for why xargs is used in answer to chepner's comment:

Imagine that there is a folder with 100 files in it. a find . -type f -exec chmod 644 {} \; will execute chmod 100 times.

Using find . -type f -print0 | xargs -0 chmod 644 execute xargs once and chmod once (or more if the argument list is very long).

This is three processes started compared to 101 processes started. The resources and time (and energy) needed to execute three processes is far less.

Edit 2: Added --no-run-if-empty as an option to xargs. Note that this may not be portable to all systems.