Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Bash Script is returning true for both but opposite string tests

Before I ran the script I have entered

# export CPIC_MAX_CONV=500

The following is the test1.script file

#!/bin/bash

function cpic () {
  var="`export | grep -i "CPIC_MAX_CONV" | awk '/CPIC_MAX_CONV/ { print $NF } '`"
  [[ $var=="" ]] && (echo "Empty String <<")
  [[ $var!="" ]] && (echo "$CPIC_MAX_CONV")
  echo "$var" ;
}

cpic

The output is:

# test1.script  ---- Me running the file

Empty String <<
500
CPIC_MAX_CONV="500"

No matter what I use "" or '' or [ or [[ the result is the same. The CPIC_MAX_CONV variable is found by the above script.

I am running this on Linux/CentOS 6.3.

The idea is simple: To find if CPIC_MAX_CONV is defined in the environment and return the value of it. If an empty space is there then of course the variable is not present in the system.

like image 561
Randhawa Avatar asked Oct 05 '22 13:10

Randhawa


2 Answers

Why do you always get true? Let's play a little bit in your terminal first:

$ [[ hello ]] && echo "True"

What do you think the output is? (try it!) And with the following?

$ [[ "" ]] && echo "True"

(try it!).

All right, so it seems that a non-empty string is equivalent to the true expression, and an empty string (or an unset variable) is equivalent to the false expression.

What you did is the following:

[[ $var=="" ]]

and

[[ $var!="" ]]

so you gave a non-empty string, which is true!

In order to perform the test, you actually need spaces between the tokens:

[[ $var == "" ]]

instead. Now, your test would be better written as:

if [[ -z "$var" ]]; then
    echo "Empty String <<"
else
    echo "$CPIC_MAX_CONV"
fi

(without the sub-shells, and with just one test).

There's more to say about your scripting style. With no offence, I would say it's really bad:

  • Don't use backticks! Use the $(...) construct instead. Hence:

    var="$(export | grep -i "CPIC_MAX_CONV" | awk '/CPIC_MAX_CONV/ { print $NF } ')"
    
  • Don't use function blah to define a function. Your function should have been defined as:

    cpic () {
        local var="$(export | grep -i "CPIC_MAX_CONV" | awk '/CPIC_MAX_CONV/ { print $NF } ')"
        if [[ -z "$var" ]]; then
            echo "Empty String <<"
        else
            echo "$CPIC_MAX_CONV"
        fi
    }
    

Oh, I used the local keyword, because I guess you're not going to use the variable var outside of the function cpic.

Now, what's the purpose of the function cpic and in particular of the stuff where you're defining the variable var? It would be hard to describe (as there are so many cases you haven't thought of). (Btw, your grep seems really useless here). Here are a few cases you overlooked:

  • An exported variable is named somethingfunnyCPIC_MAX_CONVsomethingevenfunnier
  • An exported variable contains the string CPIC_MAX_CONV somewhere, e.g.,

    export a_cool_variable="I want to screw up Randhawa's script and just for that, let's write CPIC_MAX_CONV somewhere here"
    

Ok, I don't want to describe what your line is doing exactly, but I kind of guess that your purpose is to know whether the variable CPIC_MAX_CONV is set and marked for export, right? In that case, you'd be better with just this:

cpic () {
    if declare -x | grep -q '^declare -x CPIC_MAX_CONV='; then
        echo "Empty String <<"
    else
        echo "$CPIC_MAX_CONV"
    fi
}

It will be more efficient, and much more robust.

Oh, I'm now just reading the end of your post. If you want to just tell if variable CPIC_MAX_CONV is set (to some non-empty value — it seems you don't care if it's marked for export or not, correct me if I'm wrong), it's even simpler (and it will be much much more efficient):

cpic () {
    if [[ "$CPIC_MAX_CONV" ]]; then
        echo "Empty String <<"
    else
        echo "$CPIC_MAX_CONV"
    fi
}

will do as well!

like image 188
gniourf_gniourf Avatar answered Oct 08 '22 01:10

gniourf_gniourf


Do you really care whether CPIC_MAX_CONV is an environment variable versus just 'it is a variable that might be an environment variable'? Most likely, you won't, not least because if it is a variable but not an environment variable, any script you run won't see the value (but if you insist on using aliases and functions, then it might matter, but still probably won't).

It appears, then, that you are trying to test whether CPIC_MAX_CONV is set to a non-empty value. There are multiple easy ways to do that — and then there's the way you've tried.

: ${CPIC_MAX_CONV:=500}

This ensures that CPIC_MAX_CONV is set to a non-empty value; it uses 500 if there previously wasn't a value set. The : (colon) command evaluates its arguments and reports success. You can arrange to export the variable after it is created if you want to with export CPIC_MAX_CONV.

If you must have the variable set (there is no suitable default), then you use:

: ${CPIC_MAX_CONV:?}

or

: ${CPIC_MAX_CONV:?'The CPIC_MAX_CONV variable is not set but must be set'}

The difference is that you can use the default message ('CPIC_MAX_CONV: parameter null or not set') or specify your own.

If you're only going to use the value once, you can do an 'on the fly' substitution in a command with:

cpic_command -c ${CPIC_MAX_CONV:-500} ...

This does not create the variable if it does not exist, unlike the := notation which does.

In all these notations, I've been using a colon as part of the operation. That enforces 'null or not set'; you can omit the colon, but that allows an empty string as a valid value, which is probably not what you want. Note that a string consisting of just a blank is 'not empty'; if you need to validate that you've got a non-empty string, you have to work a little harder.


I'm not dissecting your misuse of the [[ command; gniourf_gniourf has provided an excellent deconstruction of that, but overlooked the simpler notations available to do what seems to be the job.

like image 37
Jonathan Leffler Avatar answered Oct 08 '22 03:10

Jonathan Leffler