0

I wrote some bash before reading this popular question. I noticed the way I was doing things doesn't show up in any of the answers so I was curious if there is a danger/bug to my logic that I am not seeing.

if [ ! $1 ] || [ $2 ]; then
    echo "Usage: only one var." && exit 1
fi

As you can see, I am testing to see if there is one and only one parameter given on the command line. As far as I have tested, this works. Can someone pass along some more knowledge to a new bash user?

Community
  • 1
  • 1
jdf
  • 699
  • 11
  • 21
  • 2
    No, it's not. Needs more quotes -- as http://shellcheck.net/ would tell you. – Charles Duffy Jun 17 '15 at 23:03
  • 1
    Also, "empty" and "unset" are two different things. – Charles Duffy Jun 17 '15 at 23:03
  • 1
    On a different topic -- using "this" in your question's title when you could just quote the idiom forces folks to click through to read your question to decide whether it's interesting, as opposed to making the title useful enough to inform the decision on its own. Down with clickbait questions! Up with useful titles! :) – Charles Duffy Jun 17 '15 at 23:05
  • 2
    Try your script with a first argument of `"foo bar baz"` or try it with a second argument of `"-n ''"`. – Etan Reisner Jun 17 '15 at 23:05
  • Thanks to whoever posted about $# - more info [here](http://www.gnu.org/software/bash/manual/bashref.html#Shell-Parameters) at 3.4.2 if you are interested in reading more about the special parameters like $# which holds the number of parameters given. – jdf Jun 17 '15 at 23:08
  • @CharlesDuffy totally my bad, I loathe clickbait so I appreciate the edit, honestly I wasn't sure what to put in title. – jdf Jun 17 '15 at 23:09
  • @KarolyHorvath I just posted here for anyone else who may have questions, I only found the link after you or someone else maybe posted it here, hadn't seen it before but it is just what I was looking for. – jdf Jun 17 '15 at 23:19
  • @jdfLemon: No, I mean the link in your Question! – Karoly Horvath Jun 17 '15 at 23:19
  • @KarolyHorvath you're totally right, I must have missed it before since I didn't know what it did. – jdf Jun 17 '15 at 23:24

3 Answers3

3
./script one '' oops

[ $2 ] will be false when $2 is the empty string.

And as that other guy points out in a comment, bash will split both strings, opening up a range of issues, including potential security holes.

Community
  • 1
  • 1
Kevin
  • 53,822
  • 15
  • 101
  • 132
  • 1
    Also, `./script '1 = 2'` is one parameter but fails. `./script '-t a'` is one parameter and fails with error, and `./script '-f .ssh/authorized_keys'` lets you see whether or not you have ssh keys set up. This is really not acceptable. – that other guy Jun 17 '15 at 23:10
  • However, `[ "$2" ]` is mostly fine. It doesn't execute anything, and it's false only if `$2` is unset (which you want), or if `$2` is set to the empty string (which you probably _also_ want). That said, for making sure that you got enough arguments, the right test is to use `$#`, as @thomas-dickey points below. – lcd047 Jun 18 '15 at 06:36
2

This is clearer:

if [ $# != 1 ] ; then
    echo "Usage: only one var."; exit 1
fi
Thomas Dickey
  • 51,086
  • 7
  • 70
  • 105
2

Things that will break your test:

  • The first parameter is empty ("")
  • The second parameter is empty ("")
  • The first or second parameter has more words ("bla bla")
  • The parameters contain something that will be interpreted by test (e.g.: -z)

It is simply less clearer than using $#. Also the mere fact that I have to think about all the corner-cases which could potentially break the code makes it inferior.

Karoly Horvath
  • 94,607
  • 11
  • 117
  • 176