[issue1279] check4progs() does not work correctly with more then one argument

[Frank Terbeck]

Frank Terbeck <ft at grml.org> added the comment:

Hi there!

Here are some thoughts:

> diff --git a/etc/grml/script-functions b/etc/grml/script-functions
> index a39e3d8..b1de8d4 100644
> --- a/etc/grml/script-functions
> +++ b/etc/grml/script-functions
> @@ -50,13 +50,19 @@ setdialog(){
>  # {{{ check for availability of program(s)
>  check4progs(){
>    local RC=''
> -  for arg in $* ; do
> -    which $arg >/dev/null 2>&1 || RC="$arg"
> +  local RTN=0
> +  local arg=''
> +  for arg in "$@" ; do

Using "$@" here is the first improvement, that really wants me to take this
patch. It's just the right thing to to[tm].

> +    \which "$arg" >/dev/null 2>&1 || RC="$arg"

I'm unsure about the portability of "\which". Maybe better

  builtin which "$arg" ...

> +    if [ -n "$RC" ] ; then
> +      echo "$RC not installed"
> +      RC=''

I think we can do completely without this pesky "$RC" variable. How about:

    builtin which "$arg" 2>&1 > /dev/null || {
        echo "$arg not installed"

...not quite done, wait for it...

> +      if [ ${RTN} -lt 255 ]; then
> +          RTN=$(( ${RTN}+1 ))
> +      fi
> +    fi

I don't think using return values up to 255 is too portable.
IIRC, bash, dash and zsh use values up to 127 and use the stuff above
to signal return from program by fatal SIGNAL.

So, I'd just return "1".

...to continue the snippet I broke off earlier:

        RTN=1; }

>    done
> -  if [ -n "$RC" ] ; then
> -     echo "$RC not installed"
> -     return 1
> -  fi
> +  return ${RTN}
>  }

Explicit "return" is also something that I like! :)

I think my remarks should work and improve portability. Please take them into
consideration.  Other than that, thanks for taking in interest into grml!

Regards, Frank

----------
files: check4progs.mod
messages: 4640, 4641, 4663
nosy: Xk2c, ft
priority: wish
status: chatting
title: check4progs() does not work correctly with more then one argument

_____________________________________
GRML issue tracker <bts at bts.grml.org>
<http://bts.grml.org/grml/issue1279>
_____________________________________