bash for-loop - syntax error: operand expected

[Frank Herberger]

Hi there,

the following bash script snippet seems to be wrong and I cannot figure out
why. I hope you can help me as I am stuck with this for several days now.


if [ -x /bin/kill ]; then
PROCESSES="`getkill_list.sh`" # returns space sep. list of PIDs
if [ -n "$PROCESSES" ]; then
for PID in ${PROCESSES}; do
echo -e " - Killing process with PID ${PID}..."
/bin/kill -9 ${PID} >/dev/null 2>&1
done
fi
fi

# check which PIDs are on mounted /spfs filesystems

LINES="`grep '/spfs' /etc/mtab`"

# $LINES will now contain:
# /dev/mapper/vg01r5-lvol0 /spfs reiserfs rw,notail 0 0
# /spfs/mnt /spmnt none rw,bind 0 0

IFS=$'\n'
for i in ${LINES}; do # <-- problem occurs right here
if [ "`echo ${i} | awk '{print $2}'`" == "/spfs" ]; then
PIDS="`fuser -m /spfs | awk -F: '{print $2}'`"
elif [ -n "`echo ${i} | awk '{print $1}' | grep '^/spfs'`" ]; then
MPS_MOUNTED="${MPS_MOUNTED} `echo ${i} | awk '{print $2}'`"
fi
done
unset IFS


The problem now is: when the script is run for the first time,
getkill_list.sh will return a number of PIDs and the script will kill them
as intended. Afterwards, when it greps the lines out of /etc/mtab, the for
loop will fail:

./myscript.sh:471: syntax error: operand expected (error token
is "/dev/mapper/vg01r5-lvol0 /spfs reiserfs rw,notail 0 0")

When I now start it immediately again, getkill_list.sh will return no PIDs
as they have already been killed previously, so no killing is done this
time and now, the for loop runs through fine. $LINES have exactly the same
contents in both cases.

Can you give me a hint what goes wrong here?

Regards, Frank

[Chris F.A. Johnson]

Post by Frank Herberger
Hi there,
the following bash script snippet seems to be wrong and I cannot figure out
why. I hope you can help me as I am stuck with this for several days now.
if [ -x /bin/kill ]; then

Why use an external command? Bash has kill built in.

Post by Frank Herberger
PROCESSES="`getkill_list.sh`" # returns space sep. list of PIDs
if [ -n "$PROCESSES" ]; then
for PID in ${PROCESSES}; do
echo -e " - Killing process with PID ${PID}..."

What's the point of the non-standard -e option? You don't have any
escape sequences in the args.

Post by Frank Herberger
/bin/kill -9 ${PID} >/dev/null 2>&1
done
fi
fi

for PID in `getkill_list.sh`
do
printf " - Killing process with PID %d...\n" "$PID"
kill -9 ${PID} >/dev/null 2>&1
done

But why not:

pidlist=`getkill_list.sh`
echo "Killing $pidlist"
kill pidlist

Post by Frank Herberger
# check which PIDs are on mounted /spfs filesystems
LINES="`grep '/spfs' /etc/mtab`"

LINES is not a good variable to use; bash sets it to the number of
lines in the current terminal.

Why not pipe grep's output directly into a loop?

Post by Frank Herberger
# /dev/mapper/vg01r5-lvol0 /spfs reiserfs rw,notail 0 0
# /spfs/mnt /spmnt none rw,bind 0 0
IFS=$'\n'
for i in ${LINES}; do # <-- problem occurs right here

There is no problem there.

Post by Frank Herberger
if [ "`echo ${i} | awk '{print $2}'`" == "/spfs" ]; then

Avoid the non-portable == in a test expression.

Post by Frank Herberger
PIDS="`fuser -m /spfs | awk -F: '{print $2}'`"

Why is this command inside a loop? Set a flag, and call it once,
outside the loop, if the flag is set,

Post by Frank Herberger
elif [ -n "`echo ${i} | awk '{print $1}' | grep '^/spfs'`" ]; then
MPS_MOUNTED="${MPS_MOUNTED} `echo ${i} | awk '{print $2}'`"
fi
done

Whenever I see a shell loop containing many external commands, I
think that the same thing can probably be accomplished with the
shell itself, or, especially when there are multiple call to awk,
that then entire loop should be replaced with an awk script. You
will gain efficiency as well as legibility with either method.

In this case, a "while read" loop can replace the calls to awk:

fuserflag=
grep '/spfs' /etc/mtab |
while read dev mntpt type opts
do
if [ "$mntpt" -eq "/spfs" ]
then
fuserflag=1
else
case $dev in
/spfs*) MPS_MOUNTED="${MPS_MOUNTED} $2" ;;
esac
fi
done

if [ -n "$fuserflag" ]
then
PIDS=`fuser -m /spfs | cut -d: -f2`
fi

Post by Frank Herberger
unset IFS

Better is:

IFS=$' \t\n' ## IFS=$( printf " \t\n" )

Post by Frank Herberger
The problem now is: when the script is run for the first time,
getkill_list.sh will return a number of PIDs and the script will kill them
as intended. Afterwards, when it greps the lines out of /etc/mtab, the for
./myscript.sh:471: syntax error: operand expected (error token
is "/dev/mapper/vg01r5-lvol0 /spfs reiserfs rw,notail 0 0")

What is line 471?

Post by Frank Herberger
When I now start it immediately again, getkill_list.sh will return no PIDs
as they have already been killed previously, so no killing is done this
time and now, the for loop runs through fine. $LINES have exactly the same
contents in both cases.
Can you give me a hint what goes wrong here?

Put a set -x in your script and foloow the execution.

Clean up the code so that it is more easily readable (and more
efficient).

[Frank Herberger]

Hi Chris,

thanks for all your input.

Post by Chris F.A. Johnson

Post by Frank Herberger
if [ -x /bin/kill ]; then

Why use an external command? Bash has kill built in.

Ok, I will correct it. This was originally a C-Shell script which I had to
port to bash...

Post by Chris F.A. Johnson

Post by Frank Herberger
echo -e " - Killing process with PID ${PID}..."

What's the point of the non-standard -e option? You don't have any
escape sequences in the args.

It used to have escape sequences which I removed. But the -e will not hurt,
will it?

Post by Chris F.A. Johnson
pidlist=`getkill_list.sh`
echo "Killing $pidlist"
kill pidlist

I simplified the for-loop before posting. One of the processes takes a very
long time to terminate (around 20 seconds), so I have to wait until it
really disappears from the process list before I continue, this is done
inside of the for loop.

Post by Chris F.A. Johnson
LINES is not a good variable to use; bash sets it to the number of
lines in the current terminal.

Ok, I will choose a better name.

Post by Chris F.A. Johnson

Post by Frank Herberger
# /dev/mapper/vg01r5-lvol0 /spfs reiserfs rw,notail 0 0
# /spfs/mnt /spmnt none rw,bind 0 0
IFS=$'\n'
for i in ${LINES}; do # <-- problem occurs right here

There is no problem there.

Yes, there is, because this is line 471.

Post by Chris F.A. Johnson
Avoid the non-portable == in a test expression.

Ok.

Post by Chris F.A. Johnson
fuserflag=
grep '/spfs' /etc/mtab |
while read dev mntpt type opts
do
if [ "$mntpt" -eq "/spfs" ]
then
fuserflag=1
else
case $dev in
/spfs*) MPS_MOUNTED="${MPS_MOUNTED} $2" ;;
esac
fi
done
if [ -n "$fuserflag" ]
then
PIDS=`fuser -m /spfs | cut -d: -f2`
fi

Thanks for the suggestion.

Post by Chris F.A. Johnson

Post by Frank Herberger
The problem now is: when the script is run for the first time,
getkill_list.sh will return a number of PIDs and the script will kill
them as intended. Afterwards, when it greps the lines out of /etc/mtab,
./myscript.sh:471: syntax error: operand expected (error token
is "/dev/mapper/vg01r5-lvol0 /spfs reiserfs rw,notail 0 0")

What is line 471?

The for loop, see above.

Post by Chris F.A. Johnson

Post by Frank Herberger
When I now start it immediately again, getkill_list.sh will return no
PIDs as they have already been killed previously, so no killing is done
this time and now, the for loop runs through fine. $LINES have exactly
the same contents in both cases.
Can you give me a hint what goes wrong here?

Put a set -x in your script and foloow the execution.

Already done, and I cannot figure out why a

for i in /dev/mapper/vg01r5-lvol0 /spfs reiserfs rw,notail 0
0\n /spfs/mnt /spmnt none rw,bind 0 0; do foo...; done

sometimes succeeds and sometimes not.

Post by Chris F.A. Johnson
Clean up the code so that it is more easily readable (and more
efficient).

First of all I will have to get it to work. :-)

Regards, Frank