Take the 2-minute tour ×
Unix & Linux Stack Exchange is a question and answer site for users of Linux, FreeBSD and other Un*x-like operating systems.. It's 100% free, no registration required.

In the following script, the first for loop executes as expected, but not the second. I do not get any error, it seems that the script just hangs.

HOME=/root/mydir

DIR=$HOME/var
DIRWORK=$HOME/Local

for f in $(find $DIR -type f); do

    lsof -n $f | grep [a-z] > /dev/null

    if [ $? != 0 ]; then
    echo "hi"       
    fi
done


for i in $(find $DIRWORK -type d -name work); do
    echo "2"
done
share|improve this question
    
What does the find command in the second loop returns? –  Eran Ben-Natan Apr 22 '13 at 6:07
    
How long does it take the find command to finish? Until then the script "hangs", of course. Even if find doesn't find anything. –  Hauke Laging Apr 22 '13 at 6:13
    
Looks like the script took much longer than expected in case of second for loop. The first for loop was echoing output pretty fast, but not the second. Any thoughts ? –  Novice User Apr 22 '13 at 7:21
    
Thought: There is nothing wrong with your script and you are not experiencing any problems. You just feel your script should behave differently, for reasons you are not telling us. –  Bananguin Apr 22 '13 at 8:59
    
@user1129682 : I was not able to understand why the second for loop was taking longer than expected. But cleared after reading the answer from lesmana –  Novice User Apr 22 '13 at 13:02

2 Answers 2

up vote 4 down vote accepted

Your script is coded in a dangerous way.

First, I assume you are using the Bash shell since you tagged it '/bash' and '/for'.

In my answer I will quote this great Bash guide, which is probably the best source to learn Bash from out there.

1) Never use a Command Substitution, of either kind, without quotes. There is a major issue here: using an unquoted expansion to split output into arguments.

Specifically speaking, this $(find $DIRWORK -type d -name work) and $(find $DIR -type f) will undergo Word Splitting, thus if find finds a file with spaces in its name, i.e. "file name", the word splitting result of Bash will pass 2 argument for the for command to iterate over, i.e. one for "file" and one for "name". In this case you want to hope that you'll get a "file: No such file or directory" and "name: No such file or directory", instead of potentially causing damage to them if they truly exist.

2) By convention, environment variables (PATH, EDITOR, SHELL, ...) and internal shell variables (BASH_VERSION, RANDOM, ...) are fully capitalized. All other variable names should be lowercase. Since variable names are case-sensitive, this convention avoids accidentally overriding environmental and internal variables.

Your $DIRWORK directory breaks that convention, and it also unquoted, thus if we let DIRWORK='/path/to/dir1 /path/to/dir2', find will look into two different directories when $DIRWORK is unquoted. The subject of using quotes is very important in Bash, so you should "Double quote" every expansion, as well as anything that could possibly contain a special character, e.g. "$var", "$@", "${array[@]}", "$(command)". Bash treats everything within 'single quotes' as literal. Learn the difference between ' and " and `. See Quotes, Arguments and you might also want to take a look at this link: http://wiki.bash-hackers.org/syntax/words

This is a safer version of your script, which I recommend you to use instead:

my_home="/root/mydir"

my_dir="$my_home/var"
dir_work="$my_home/Local"

while IFS= read -r -d '' f; do
    # I'm guessing that you also want to ignore stderr;
    # this is where the 2>&1 came from.
    if lsof -n "$f" | grep '[a-z]' > /dev/null 2>&1; then
        echo "hey, I'm safer now!"
    fi
done < <(find "$dir_work" -type f -print0)


while IFS= read -r -d '' f; do
    echo "2"
done < <(find "$dir_work" -type d -name 'work' -print0)

As you can see, the IFS variable is set to be emtpy, thus preventing read from trimming leading and trailing spaces from a line. The read command uses an empty string ( -d '' ) as a delimiter, to read until it reaches a \0. find needs to be modified accordingly, therefore it uses the -print0 option to delimit its data with a \0 instead of a new line - which, amazingly and maliciously, can be a part of a file name. Splitting such a file by \n into two pieces will break our code.

You might want to read about Process Substitution if you don't understand my script completely.

The previous answer which stated that find ... | while read name; do ...; done should be used for reading finds output can be also bad. The while loop above is executed in a new subshell with its own copy of variables copied from the parent. This copy then is used for whatever you like. When the while loop is finished, the subshell copy is discarded, and the original variables of the parent did not change.

If you aim on modifying some variables inside this while loop and use them afterwards in the parent, consider using the safer script above which will prevent data loss.

share|improve this answer
    
"Never use a Command Substitution, of either kind, without quotes." This is just nitpicking, but it's okay to use command substitution without quotes when you're setting a variable: something=$(basename "filename with spaces"). –  paraxor Apr 22 '13 at 21:43
    
Thank you for improving my answer. –  Rany Albeg Wein Apr 22 '13 at 23:19

This code

for i in $(find $DIRWORK -type d -name work); do
    echo "2"
done

will first execute this line

find $DIRWORK -type d -name work

wait until find finished executing and then take the output and put it back in the for loop

for i in the output of find; do
    echo "2"
done

only then the for loop will start executing.

So if the find is taking a long time to finish the for loop has to wait a long time before it can start.

Try timing the find command in an interactive prompt

$ time find $DIRWORK -type d -name work

and see how long that takes.


Also note: you shouldn't use a for loop to loop over filenames. Use a while loop with read like this:

find $DIRWORK -type d -name work | while read name; do
    echo "2"
done

Read this for more information.

Bonus: this executes the while loop in parallel to find. That means the while loop will execute one iteration as soon as find prints out one line. It does not have to wait for find to finish executing.

share|improve this answer
    
Thanks. Exactly what i was looking for. –  Novice User Apr 22 '13 at 13:01

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.