How to use ${OPTARG} on getopts?

Marko picture Marko · Mar 27, 2016 · Viewed 7.2k times · Source

I have the following code:

while getopts ":p:t:n:" o; do
case "${o}" in
    p)
        p=${OPTARG}
        numep=$p
        mkdir $numep
        ;;
    t)
        t=${OPTARG}
        tip=$t
        if [ $tip == "c" ]; then
            touch $numep/$numep.c
            touch $numep/$numep.h
        elif [ $tip == "c++" ]; then
            touch $numep/$numep.cpp
            touch $numep/$numep.h
        fi
            ;;
        n)
            n=${OPTARG}
            nrFis=$n
            if [ $tip == "c" ]; then
                for i in $(seq $n); do
                    touch $numep/src/$numep$i.c
                    touch $numep/inc/$numep$i.h
                done
            elif [ $tip == "c++" ]; then
                for i in $(seq $n); do
                    touch $numep/src/$numep$i.cpp
                    touch $numep/inc/$numep$i.h
                done
            fi
            ;;
        *)
            err-help
            ;;
        esac
    done

err-help is a previously defined function that delivers instructions for a correct call.

A correct call of the script looks like this: ./script.sh -p project_name -t project_type -n source_files_number

What am I trying to do: create a directory with a specific number of source files for a c or a c++ project.

My questions here: How do I have to use the ${OPTARG} ? What exactly does the p=${OPTARG} thing? Will the p, t, n variables fill with the right content? How am I doing this right, please? :(

Answer

glenn jackman picture glenn jackman · Mar 27, 2016

You are using getopts correctly, but you are making assumptions about what order you're going to see the options. More on this later.

Let's look at a couple of excerpts from help getopts

OPTSTRING contains the option letters to be recognized; if a letter is followed by a colon, the option is expected to have an argument, which should be separated from it by white space.

... When an option requires an argument, getopts places that argument into the shell variable OPTARG.

This is the magic of getopts. Since you added p: into your optstring, when $0 = "p", the variable $OPTARG will contain whatever you provided to -p, in this case the string "project_name".

Now for some code review:

case "${o}" in
    p)
        p=${OPTARG}
        numep=$p
        mkdir $numep
        ;;

You only need to use braces if you are using arrays, or you need so disambiguate the variable name from the surrounding text ($o_x vs ${o}_x) It would be tidier to write

case "$o" in ...

You don't need to create a special variable with the same name as the option. You never use $p. You can just write

numep="$OPTARG"

Always quote your variables. Always. Unless you specifically know when to leave them unquoted. If I provide -p "some dir with spaces", then mkdir $numep will create 4 directories. You need to write

mkdir "$numep"

If the $numep directory already exists, you'll get an error message. If you don't want that, use mkdir -p "$numep" (see man mkdir)

Now, do address my first concern: there's nothing wrong with invoking your script and providing the options in any order

./script.sh -t project_type -n source_files_number -p project_name

Your option parsing assumes, in the t and n branches, that you've already set $numep, so you're assuming that the user provided -p first. If -t comes first, then $numep is empty, and you will attempt to create the file /.h. Even if you have permission to write to the root directory, this is not what you want to do. Parse all your options before you start doing stuff.

I would write this:

while getopts ":p:t:n:" o; do
    case "$o" in
        p)  numep="$OPTARG" ;;
        t)  case "$OPTARG" in
                c)  ext="c" ;;
                "c++")  ext="cpp" ;;
                *)  echo "Error: use 'c' or 'c++' for -t option" >&2
                    exit 1
                    ;;
            esac
            ;;
        n)  nrFis="$OPTARG" ;;
        *)  err-help ;;
    esac
done

mkdir -p "$numep"
touch "$numep/$numep.$ext"
touch "$numep/$numep.h"
for ((i=1; i<=$nrFis; i++)); do
    touch "$numep/src/$numep$i.$ext"
    touch "$numep/inc/$numep$i.h"
done