mjhjmtu

I'm taking a look at the optparse library for bash option parsing, specifically this bit in the generated code:

params=""
while [ $# -ne 0 ]; do
 param="$1"
 shift

 case "$param" in

 --my-long-flag)
 params="$params -m";;
 --another-flag)
 params="$params -a";;
 "-?"|--help)
 usage
 exit 0;;
 *)
 if [[ "$param" == --* ]]; then
 echo -e "Unrecognized long option: $param"
 usage
 exit 1
 fi
 params="$params "$param"";; ##### THIS LINE
 esac
done

eval set -- "$params" ##### AND THIS LINE

# then a typical while getopts loop

Would there be any real reason to use eval here? The input to eval seems to be properly sanitized. But wouldn't it work the same to use:

params=()
# ...
 --my-long-flag)
 params+=("-m");;
 --another-flag)
 params+=("-a");;
# ...
 params+=("$param");;
# ...
set -- "$params[@]"

That seems cleaner to me.

In fact, wouldn't this allow options to be parsed directly out of the params array (without even using set) by using while getopts "ma" option "$params[@]"; do instead of while getopts "ma" option; do?

You don't need to use a bash array here (but do so if it feels better).

Here's how to do it for /bin/sh:

#!/bin/sh

for arg do
 shift

 case "$arg" in
 --my-long-flag)
 set -- "$@" -m ;;
 --another-flag)
 set -- "$@" -a ;;
 "-?"|--help)
 usage
 exit 0 ;;
 --*)
 printf 'Unrecognised long option: %sn' "$arg" >$2
 usage
 exit 1 ;;
 *)
 set -- "$@" "$arg"
 esac
done

This is cleaner than the bash array solution (personal opinion) since it doesn't need to introduce another variable. It's also better than the auto-generated code that you show as it retains each command line argument as a separate item in "$@". This is good, because this allows the user to pass arguments containing whitespace characters as long as they are quoted (which the auto-generated code does not do).

Style comments:

• The loop above is supposed to translate long options into short options for a loop over getopts later. As such, it breaks from that task by actually acting on some options, such as -? and --help. IMHO, these should instead be translated to -h (or some suitable short option).



• 
  

  It also does the translation of long options past the point where options should not be accepted. Calling the script as

  
  
  

  ./script.sh --my-long-flag -- -?
  

  
  
  

  should not interpret -? as an option due to the -- (meaning "options ends here"). Likewise,

  
  
  

  ./script.sh filename --my-long-flag
  

  
  
  

  should not interpret --my-long-option as an option, as the parsing of options should stop at the first non-option.

  
  

Here's a variant that takes the above into account:

#!/bin/sh

parse=YES

for arg do
 shift

 if [ "$parse" = YES ]; then
 case "$arg" in
 --my-long-flag)
 set -- "$@" -m ;;
 --another-flag)
 set -- "$@" -a ;;
 --help)
 set -- "$@" -h ;;
 --)
 parse=NO
 set -- "$@" -- ;;
 --*)
 printf 'Unrecognised long option: %sn' "$arg" >$2
 usage
 exit 1 ;;
 *)
 parse=NO
 set -- "$@" "$arg"
 esac
 else
 set -- "$@" "$arg"
 fi

done

What this does not allow is long options with separate option arguments, such as --option hello (the hello would be treated as a non-option and the option parsing would end). Something like --option=hello would be fairly easy to handle with a bit of extra tinkering though.

The question is "Should a bash array be used in place of eval set — “$params”?", and the answer is yes!.

In your script, the input to eval is clearly not properly sanitized. Try

 yourscript '`xterm`'

and you'll see that an xterm is started even though the backticks are properly quoted by single quotes. (Compare with

 echo '`xterm`'

which does not start an xterm.)

Fixing the bug while keepting eval is very difficult.
Even changing the line

 params="$params "$param"";;

to

 params="$params '$param'";;

would not help: Now

 yourscript '`xterm`'

no longer starts an xterm, but

 yourscript '' `xterm` ''

still does.