Bash script: binary operator expected

Question

So I have reviewed a lot of entries on here and cannot figure out what I am doing wrong here. I am new to scripting and want to know why this does not work:

input is

./filedirarg.sh /var/logs fileordir.sh

script is

#! /bin/bash
echo "Running file or directory evaluation script"
LIST=$@
if [ -f $LIST ]
then
echo "The entry ${LIST} is a file"
elif [ -d $LIST ]
then
echo "The entry ${LIST} is a directory"
fi

This results in

./filedirarg.sh: line 4: [: /var/logs: binary operator expected
./filedirarg.sh: line 7: [: /var/logs: binary operator expected

It runs fine with this

./filedirarg.sh fileordir.sh 

Quoting the $LIST in the evaluation results in no output except the first echo statement.

Solution

I think [ -f … ] (or test -f …) requires exactly one argument. When you run

./filedirarg.sh /var/logs fileordir.sh

there are two. The same with [ -d … ].

---

This is a quick fix:

#! /bin/bash
echo "Running file or directory evaluation script"

for file ; do
 if [ -f "$file" ]
 then
  echo "The entry '$file' is a file"
 elif [ -d "$file" ]
 then
  echo "The entry '$file' is a directory"
 fi
done

Thanks to quoting it should work with names with spaces (e.g. ./filedirarg.sh "file name with spaces").

Also note for file ; do is equivalent to for file in "$@" ; do.

Comments

[Scott - Слава Україні]

(1) Stupid question: you show a command that mentions two scripts, and then you show a script.  Which script are you showing?  I guess it’s obvious, but it’s better if you say. (2) Please describe what the script is supposed to do, rather than forcing us to read it and try to figure it out (given that you know that it’s wrong). (3) Have you posted the script accurately (e.g., by copy-and-paste) or did you just look at it and retype it?  [ -f $LIST } would cause an error (mismatched brackets). (4) Please indent your code intelligently.  … (Cont’d)

[Scott - Слава Україні]

(Cont’d) …  (5) You need to learn how to debug. There are lots of resources for that here and elsewhere on the web. Simple things to try: echo "$LIST", and add an else clause to your if.

[seolchan]

Thanks for saying my question is stupid first off. Much appreciated when I am just starting out. Second, you can clearly see that filedirarg is the script being executed with another script name as one of the arguments being passed. I couldn't copy the script directly due to restricted access and had to type this out. The } is a typo. The script has ]. And elif should work fine and does with a file name instead of a directory. Not very helpful.

[Kamil Maciorowski]

This addresses your comment under David's answer. I write it here because it has nothing to do with the answer. You wrote "LIST=$@ accepts all args and works fine". No, it doesn't work fine. If you run ./script.sh a "b c" then "$@" expands to two items and almost always this is what you want, $@ expands to three items. Now take LIST=$@. Then $LIST expands to three items as well, "$LIST" expands to one item. Work with "$@", rewriting to another name brings you trouble. Also it's a good practice to name your variables in lowercase.

[seolchan]

So the reason I am doing variable names in upper case is that the instructor in the class I am taking literally said that it was best practice to write variable names in upper case. He is also where I got the idea of setting a variable to $@ to take in every argument instead of specifying them like $1 $2. I guess that is incorrect? And you are correct it is not needed. I was trying to expand LIST to 3 different items, then run each item through the evaluations. I am now more confused. :-(

[Kamil Maciorowski]

"$@" is perfectly correct where you can pass multiple arguments. Sometimes you can't, like with your test -f … or so. My point is: there's no need to create another variable with the same content as "$@" because the content is not really the same. Check my updated answer to see there is "$@" used there but it's hidden under the default behavior of for some_variable ; do.

[Scott - Слава Україні]

(1) I never meant to say that your question was stupid.  I see now that I wasn’t clear; I’m sorry about that.  I mean that, by asking you “Which script are you showing?”, I was asking a stupid question.  Because, yes, I can clearly see that filedirarg is the script being executed with another script name as one of the arguments — and I could also clearly see that line 4 said [ -f $LIST }; that which is obvious is not always true. I still believe that you should say that the script is filedirarg.sh, and/or change the fileordir.sh argument … (Cont’d)

[Scott - Слава Україні]

(Cont’d) …  to an_unrelated_script.sh, or, better yet, a_plain_file.  (2) I still strongly recommend that, when you ask a question like this, you describe what the script is supposed to do.  I’m still wondering what you were expecting to happen when you passed multiple arguments to a script that doesn’t have the capability to loop.  Given that your if-then-elif structure allows only one echo statement to execute, what output were you expecting?  (4) I repeat my request that you indent code to illuminate its structure. … (Cont’d)

[Scott - Слава Україні]

(Cont’d) …  I’m disappointed that, even though you edited your question after you saw my comment(s), you fixed only the typo, and didn’t address the other issues. (5) When I suggested that you add an else I meant after the echo "The entry ${LIST} is a directory" statement. If you had gotten a message, The entry /var/logs fileordir.sh is neither a file or a directory, wouldn’t that have helped you to understand what was happening? … (Cont’d)

[Scott - Слава Україні]

(Cont’d) …  (11) I echo @Kamil’s advice to name your variables in lower case, but, if your instructor tells you to write variable names in upper case, then you should do that.  (12) If your instructor tells you to set a variable to $@, you should get a better instructor.  (13) You say (in a comment on David’s answer, but addressed to me) that “LIST=$@ accepts all args and works fine”. … (Cont’d)

[Scott - Слава Україні]

(Cont’d) …  I invite (challenge) you to post a complete script that uses LIST=$@ and does something useful, like doing something with each argument (individually). Setting LIST and then never using it doesn’t count.  echo "$LIST" doesn’t count. Check whether it works with names that contain spaces, like Program files or shopping list.

[DavidPostill]

There is also another more obvious error :)

[Kamil Maciorowski]

@DavidPostill I know. I assumed it's a typo because if you actually run it with } it throws a different error.

[Scott - Слава Україні]

There is also another more obvious error: LIST=$@

[AFH]

The for line could be simplified to for file; do. Your in clause is the default when it is omitted.

[seolchan]

Like I said above, that was a typo due to me having to write it out. Thanks though.

[Kamil Maciorowski]

@AFH Thank you. I've learnt something today. :)

[seolchan]

and Scott, LIST=$@ accepts all args and works fine ;-)

[seolchan]

Added the file=$@ at the top to accept the arguments. This works perfectly now. Thanks!

[seolchan]

All the useful assistance is VERY much appreciated. Everyone here but Scott helped me expand my knowledge.

[Kamil Maciorowski]

@seolchan "Added the file=$@ at the top to accept the arguments." – Frankly it seems to make no sense. My script works for me without this modification.

[AFH]

@KamilMaciorowski - In passing, it is worth noting that going through an intermediate variable (LIST in the original question) will always cause problems when there are spaces in the file names. By going direct to the argument list, as you have done, preserves the spaces, though it is not obvious that you have solved this issue at the same time. I have not found a variant on LIST="$@" which preserves the correct file names on expansion, except by using an array: list=("$@"), then for f in "${list[@]}" .... The questioner certainly has raised some interesting issues!

[AFH]

@KamilMaciorowski - I see I have just crossed with your comment under the original question!

[DavidPostill]

@seolchan Then next time please take care to copy and paste the code you actually using. Otherwise you are wasting our time :/