BTS

Message4663

Author ft
Recipients Xk2c
Date 2013-11-06.09:54:49
Content
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
History
Date User Action Args
2013-11-06 09:54:49ftsetrecipients: + Xk2c
2013-11-06 09:54:49ftsetmessageid: <1383731689.42.0.0221295832866.issue1279@bts.grml.org>
2013-11-06 09:54:49ftlinkissue1279 messages
2013-11-06 09:54:49ftcreate