60

I have written a function:

check_log(){
    if [ -f "/usr/apps/appcheck.log" ]
    then
         return 1
    else
         return 0
    fi
}

Then I call this function in an "if" condition:

if [ check_log ];
then
    ........statements....
fi

Will this work? I am confused here because bash returns 0 on success and 1 on failure, but my function is returning 1 and the condition is checking for 1/0, it gets 1 and it should give failures, but in my shell script the condition is passing.

Can anyone shed light on this issue?

Improve this question
3
  • What about a number if [ any number ] do something and if [ 0 ] do nothing Commented Mar 27, 2013 at 22:55
  • In bash, you'd use a numeric context for that -- not [ ] or [[ ]] but (( )) -- like so: if (( variable )); then ... will evaluate to true only if variable contains a number more than 0. Commented Mar 28, 2013 at 2:12
  • ...by the way, when "using bash", be sure your shebang is #!/bin/bash or #!/usr/bin/env bash, not #!/bin/sh; if it's the latter, not all bash features will be enabled, so not all advice given here will be valid. Commented Mar 28, 2013 at 2:35

2 Answers 2

Reset to default
91 
if [ check_log ];

When you use square brackets you're invoking the test command. It's equivalent to if test check_log which is shorthand for if test -n check_log, which in turn means "if "check_log" is not an empty string". It doesn't call your check_log function at all.

Change it to this:

if check_log;

By the way, the function could be more simply written as:

check_log() {
    ! [ -f "/usr/apps/appcheck.log" ]
}

The return value from a function is the exit status of the last command, so no need for explicit return statements.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

What about a number if [ 6 ] do something and if [ 0 ] do nothing
@JumpOffBox If you want to do numeric tests, you have to use one of the test command's numeric operators, like if [ 6 -ne 0 ]; then .... But note that you cannot use a function in place of one of those numbers; you should think of a shell function as a command (that can succeed or fail), not like a mathematical function (whose primary purpose is to return a value).
@GordonDavisson Please don't suggest [ ] for someone explicitly targeting bash rather than /bin/sh; (( 7 != 0 )) is far more readable (and also less error-prone; (( )), like [[ ]], suppresses string-splitting and glob expansion, so quotes aren't necessary to ensure that statements are parsed as intended even with unexpected data values).
So in essence if check_log; will check if the return value is false (0) which means "success".
I wouldn't call 0 "false". Exit codes aren't booleans. It's better to think of them as error codes. There's one way to succeed but many ways to fail so we encode success as 0 and failures as anything else.
5

As noted by @john-kugelman, one solution (and perhaps the most correct one) is to use the following syntax:

if check_log;

But an alternative solution is:

if [[ $(check_log; echo $?) -eq 0 ]];

My personal preference is the latter as it fosters consistency among conditional statements. But the downside is that because it relies on command substitution, it will fork a child process (i.e. a subshell) where as the first method will not.

An interesting read on the subject can be found here:

When does command substitution spawn more subshells than the same commands in isolation?

Updated (2020-12-01): In most cases it is inadvisable to use the above alternative solution for reasons related to inefficiency and the potential to trigger erroneous results especially if the called function writes to stderr or stdout and that output isn't trapped.

Updated (2017-10-20): Strikeout my preference for the second method as these days I'm on a mission to prevent unnecessary forking. The syntax in the first solution is not as intuitive for non-shell programming, but it certainly is more efficient.

Improve this answer

5 Comments

Why in the world would you prefer the second one?
If you no longer advise the practices given in this answer, you might consider deleting it. We've had new questions asked by people trying to apply this advice, and it would be better if they didn't.
@CharlesDuffy I don't think it's best practice to actually delete answers that others may have bookmarked. The alternative solution is still correct in syntax it just happens to fork a subshell for evaluation which may or may not be desirable. Spawning subshells is a lightweight process and the pattern is prevalent.
The answer is not just inefficient, it's also buggy. If check_log writes to stdout, anything it writes ends up on the left-hand side of your -eq and causes the test to fail, no matter what the actual exit status is. There is never a case where this is good practice -- even if you want to explicitly test $? (say, to be in a specific range), it's still better to do use different syntax -- such as if check_log; [ "$?" -lt 3 ]; then.
@CharlesDuffy Interesting. I didn't consider the case that there might be output to stdout (check_log doesn't do this). Redirecting output could address that case if [[ $(check_log > /dev/null; echo $?) -lt 3 ]]; then but still inefficient and prone to the problem you suggest. As to never a case I think that's a bit harsh, perhaps never in your use cases or perhaps many others would be more reasonable. Updating the answer to steer folks in another direction.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.