syntax error near unexpected token `then'

Question

I created bash script wich works as Nagios plugin. Here is a part of it:

44  CURDUPLEX=`ethtool $IFACE|grep "Duplex"|cut -d ':' -f 2|xargs`
45  CURSPEED=`ethtool $IFACE|grep "Speed"|cut -d ':' -f 2|xargs|sed 's/.\{4\}$//'`
46  
47  if [ $CURSPEED -eq $SPEED ]; then
48          if [ $CURDUPLEX = $DUPLEX ]; then
49                  echo "OK: Interface $IFACE link at ${CURSPEED}Mb\s and $CURDUPLEX duplex."
50                  exit $OK
51          else
52                  echo "CRITICAL: Interface $IFACE link at ${CURSPEED}Mb\s [OK] and $CURDUPLEX duplex (Expected $DUPLEX)."
53                  exit $CRIT
54          fi
55  elif [ $CURDUPLEX = $DUPLEX ]; then
56          echo "CRITICAL: Interface $IFACE link at ${CURSPEED}Mb\s and $CURDUPLEX duplex [OK]."
57          exit $CRIT
58  else
59          echo "CRITICAL: Interface $IFACE link at ${CURSPEED}Mb\s and $CURDUPLEX duplex (Expected $DUPLEX)."
60          exit $CRIT
61  fi
62  }

When I run it I see the error:

]# ./check_physlink usage
 ./check_physlink: line 48: syntax error near unexpected token `then'
 ./check_physlink: line 48: `        if [ $CURDUPLEX = $DUPLEX ]; then'

I think about for an hour yet, and cannot to understand what is wrong. Thought about no space between left bracket and dollar sign, but no as you see.

Please advise. Thank you.

`=` is used for string comparison so you should probably be double quoting your variables as strings (especially to prevent any funkiness in edge cases with whitespace). — Garrett, Jul 27 '12 at 20:19
Is the $DUPLEX variable defined when you run `./check_physlink usage`? As gman said, you need to better handle the comparison strings inside the brackets, likely by using double quotes. Take a look at http://mywiki.wooledge.org/BashPitfalls/ in general, and, say, #4 in particular. — cjc, Jul 27 '12 at 21:07

score 2 · Answer 1 · answered Jul 31 '12 at 14:19

2

Since you're writing for bash, you should really use [[, not [, for tests. Oh, and USE MORE QUOTES!

Here are some guide and FAQ entries to get you started on improving the script:

Why `` is bad

Using quotes

General practices

answered Jul 31 '12 at 14:19

adaptr

16,576
23
34

1

With just `[` you will need quotes. With `[[`, they are optional, unless you want to escape wildcards. – Henk Langeveld Aug 14 '12 at 08:47
shopt -s -o nounset is also very helpful, it warns you when a variable is not set e.g. really good at catching spelling type errors. – gm3dmo May 02 '13 at 15:50

Henk Langeveld · Accepted Answer · 2014-01-22T20:36:09.240

Why the xargs? I had to look it up in the POSIX spec to be reminded that it defaults to echo, but I rarely see it used that way. The Gnu man page doesn't even mention that.

44  CURDUPLEX=`ethtool $IFACE|grep "Duplex" ...`
45  CURSPEED=`ethtool $IFACE|grep "Speed" ...`

Calling ethtool twice gives Nagios performance a bad name.

Try something like:

toolsays=$( ethttool $IFACE )
CURDUPLEX=$( echo "$toolsays" | grep Duplex ... )
CURSPEED=$( echo "$toolsays" | grep Speed ... )

Then, the original script cuts of the last four characters from speed, but later on the code still assumes "Mb/s". Make that assumption explict, so the script will fail when it encounters a host where ethtool does not report the speed in Mb/s.

You can go a step further, and replace the two pipelines functions like:

function duplex_state { typeset toolsaid=$1 s=
    : ${toolsaid:?}
    s=${toolsaid##*Duplex: }
    echo ${s%%$'\n'*}
}
function speed_value { typeset toolsaid=$1 s=
    : ${toolsaid:?}
    s=${toolsaid##*Speed: }
    echo ${s%%Mb/s*}
}

And here's how you call them:

CURDUPLEX=$( duplex_state "$toolsays" )
CURSPEED=$( speed_value "$toolsays" )

Note how the captured output from ethtool still needs to be quoted when not used in an assignment to preserve any linebreaks.

syntax error near unexpected token `then'

2 Answers2