4
\$\begingroup\$

I have a bash script that reads a config file with key/value pairs and uses these values to modify the functioning of other scripts correspondingly.

Example .cfg file:

layout='true'
code_compliance='true'
....
....
variablename='false'
output='true'

The file consists of a very less amount of lines currently but there might be a chance of increase in the size of the file in the future.

I have written code that reads the file line by line and gets the key value pairs. Is it better (in terms of performance such as speed, memory requirements and coding standards) to code this way or to search for the key strings and find its corresponding value using grep?

#!/bin/bash

###########################################################################################################################
#This script is responsible for reading inputs from the configuration file and running corresponding scripts.
#If the value for an option is set to true, then the suitable script is run.
#If the user provides an invalid value for an option, an error message is displayed.
#Likewise, if an invalid option is provided, an error message is displayed.
###########################################################################################################################


SCRIPT=`readlink -f $0`
SCRIPTPATH=`dirname $SCRIPT`
TCPATH=$(echo $SCRIPTPATH | sed 's/\/bld.*//')
dir=$TCPATH/bld/code_compliance


# Define the colours used for interface output.
source $TCPATH/bld/common/def_colours

echo "VF Code Compliance Tests: $(date)" > $3
echo "" >> $3

while read line2
do
name2=$line2
name2=${name2^^}
if [[ $name2 =~ "=" && $name2 =~ "OUTPUT" ]]; then
    option2=${name2%=*}
    value2=${name2#*=}
    start2=${value2#*\'}
    end2=${start2%\'*}
    output=$end2
fi
done < $2


while read line
do
name=$line
name=${name^^}

    if [[ $name =~ "=" ]]; then

        option=${name%=*}
        value=${name#*=}
        start=${value#*\'}
        end=${start%\'*}


        if [[ $output == "TEXTFILE" ]]; then

            if [[ "$end" == "TRUE" ]]; then

            case $option in

                VARIABLENAME*) 
                        bash $dir/varname $1 >> $3 2> /dev/null
                        sed -r -i "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" $3 
                        ;;

                CLASSSTRUCTNAME*) 
                        bash $dir/classnamecheck $1 >> $3 2> /dev/null
                        sed -r -i "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" $3 
                        ;;

                CONSTNAME*) 
                        bash $dir/constcheck $1 >> $3 2> /dev/null
                        sed -r -i "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" $3
                        ;;

                FUNCNAME*) 
                        bash $dir/funcnamecheck $1 >> $3 2> /dev/null
                        sed -r -i "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" $3
                        ;;

                FILECOMPLETE*) 
                        bash $dir/filecomplete $1 >> $3 2> /dev/null
                        sed -r -i "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" $3
                        ;;

                HEADERGUARD*) 
                        bash $dir/headerguard $1 >> $3 2> /dev/null
                        sed -r -i "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" $3
                        ;;

                FILENAME*) 
                        bash $dir/filenamesplchar $1 >> $3 2> /dev/null
                        sed -r -i "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" $3
                        ;;

                HEADERFILECHECK*) 
                        bash $dir/headercheck $1 >> $3 2> /dev/null
                        sed -r -i "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" $3
                        ;;

                LAYOUT*) 
                        bash $dir/layout $1 >> $3 2> /dev/null
                        sed -r -i "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" $3
                        ;;

                CODE_COMPLIANCE*)
                        ;;

                OUTPUT*)
                        ;;

                *) 
                        echo -e "${RED}Invalid option ${CYAN}$option ${RED}for value ${CYAN}$end${NO_COLOUR}" ;;

            esac


            elif [[ "$end" == "FALSE" ]]; then
                :

            elif [[ "$end" == "TEXTFILE" || "$end" == "CONSOLE" ]]; then
                :

            else
                echo -e "${RED}Invalid value ${CYAN}$end ${RED}for option ${CYAN}$option${NO_COLOUR}"
            fi

        elif [[ $output == "CONSOLE" ]]; then

            if [[ "$end" == "TRUE" ]]; then

            case $option in

                VARIABLENAME*)      bash $dir/varname $1 ;;

                CLASSSTRUCTNAME*)   bash $dir/classnamecheck $1 ;;

                CONSTNAME*)         bash $dir/constcheck $1 ;;

                FUNCNAME*)      bash $dir/funcnamecheck $1 ;;

                FILECOMPLETE*)      bash $dir/filecomplete $1 ;;

                HEADERGUARD*)       bash $dir/headerguard $1 ;;

                FILENAME*)      bash $dir/filenamesplchar $1 ;;

                HEADERFILECHECK*)   bash $dir/headercheck $1 ;;

                LAYOUT*)        bash $dir/layout $1 ;;

                CODE_COMPLIANCE*)   ;;

                OUTPUT*)        ;;

                *) 
                            echo -e "${RED}Invalid option ${CYAN}$option ${RED}for value ${CYAN}$end${NO_COLOUR}" ;;

            esac

            elif [[ "$end" == "FALSE" ]]; then
                :

            else
                echo -e "${RED}Invalid value ${CYAN}$end ${RED}for option ${CYAN}$option${NO_COLOUR}"
            fi

        elif [[ $output == "BOTH" ]]; then

            if [[ "$end" == "TRUE" ]]; then

            case $option in

                VARIABLENAME*)      bash $dir/varname $1 | tee -a $3 ;;

                CLASSSTRUCTNAME*)   bash $dir/classnamecheck $1 | tee -a $3 ;;

                CONSTNAME*)     bash $dir/constcheck $1 | tee -a $3 ;;

                FUNCNAME*)      bash $dir/funcnamecheck $1 | tee -a $3 ;;

                FILECOMPLETE*)      bash $dir/filecomplete $1 | tee -a $3 ;;

                HEADERGUARD*)       bash $dir/headerguard $1 | tee -a $3 ;;

                FILENAME*)      bash $dir/filenamesplchar $1 | tee -a $3 ;;

                HEADERFILECHECK*)   bash $dir/headercheck $1 | tee -a $3 ;;

                LAYOUT*)        bash $dir/layout $1 | tee -a $3 ;;

                CODE_COMPLIANCE*)   ;;

                OUTPUT*)        ;;

                *) 
                            echo -e "${RED}Invalid option ${CYAN}$option ${RED}for value ${CYAN}$end${NO_COLOUR}" ;;

            esac

            elif [[ "$end" == "FALSE" ]]; then
                :

            else
                echo -e "${RED}Invalid value ${CYAN}$end ${RED}for option ${CYAN}$option${NO_COLOUR}"
            fi
        fi
    fi
done < $2
if [[ $output == "TEXTFILE" ]]; then
echo -e "\n${BOLD_GREEN}Code Compliance has been successfully completed and the output is stored as a text file in $3 ${NO_COLOUR}\n"
fi
if [[ $output == "CONSOLE" ]]; then
echo -e "\n${BOLD_GREEN}Code Compliance has been successfully completed and the output is printed in the console${NO_COLOUR}\n"
fi

exit 0

I have to read the option output from the .cfg file whether to print the output to console or to a file or to both. Are there any better ways to do it rather than having multiple case statements and if conditions?

\$\endgroup\$
2
\$\begingroup\$

I guess you are over-engineering. The .cfg file follows bash syntax. You may just source it, and all the options get into variables automagically:

vnp@home ~/projects/play/cr/foo $ cat foo.cfg
output='true'

vnp@home ~/projects/play/cr/foo $ cat foo
source foo.cfg
echo $output

vnp@home ~/projects/play/cr/foo $ ./foo
true
vnp@home ~/projects/play/cr/foo $
\$\endgroup\$
  • 2
    \$\begingroup\$ What you suggest is indeed much simpler, and is common practice. It should be noted, though, that you should only use that technique if the file is only editable by non-hostile users. \$\endgroup\$ – 200_success Dec 22 '14 at 4:27
  • \$\begingroup\$ @200_success This is certainly correct. I presumed that in the given context security is least of the worries. \$\endgroup\$ – vnp Dec 22 '14 at 4:52
4
\$\begingroup\$

While I don't believe that . sourcing a user-edited configuration file is a good way to go - executing arbitrary code is usually a bad idea - I do wholeheartedly agree with the other answer here that you have overengineered this solution.

Here's an example:

...
while read line
do
name=$line
name=${name^^}

    if [[ $name =~ "=" ]]; then

        option=${name%=*}
        value=${name#*=}
        start=${value#*\'}
        end=${start%\'*}
...

In the first place it's important to remember that read line does not get a line of input into the shell variable $line but rather an interpretation of that line of input is stored in $line. By default any leading or trailing whitespace is trimmed from the input and \\backslash escapes are interpreted - regardless of any 'single- or "double-quote characters. To reliably put a line of input into $line with the shell builtin read you need to do:

... IFS= read -r line ...

That said, I think, based on the next bit there, you could do with a little field splitting here:

... IFS== read -r name val ...

Of course, such a thing depends on 1 $name + $val pair per line and it still doesn't validate $name. A portable shell name can consist of only alphanumeric characters and/or a _ character and cannot begin with a numeric character. What's more, even if you verify your $name to within that range - you must still ensure it is not a $name that your currently running shell already depends upon. If you uppercase path= with ${name^^}, for example, to PATH and then set that variable you're going to have a bad time - or, worse, your user will. Consider prepending a _ to the beginning of every var name set as a result of evaling your cfg.

Ok, so, all that said, you can go a long way toward doing this correctly with something like:

set -f; IFS='
'; for v in $( sed "/^[_[:alpha:]][_[:alnum:]]*=/!d
                    s/'"'/&\\&&/g ; s/=/&'"'/ 
                    s/.*/_&'/" config.cfg
); do   set "${v%%=*}" "$@" && eval "$v"
done; set +f

...though that still has several problems, it gets a list of all varnames pulled out of config.cfg and puts them in "$@". It safely evals them and sets the variables as well - each var= modified to a _var= - and safely handles all quoting. It handles only a single line per var value though, and if your file looks like:

var='value'

...it saves the hardquotes into the value literally. It is better to leave the shell quotes out of the config file though - let the values be literal there - in that way it will be easy to handle...

var=somebody's value

...so you needn't rely on your users to do:

var='somebody'\''s value'

...because a great many of them will likely not do that correctly.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.