[MPlayer-dev-eng] [PATCH] cleanup faterun.sh, do not use / if FATE_SAMPLES is not set

[Alexander Strasser]

Hello Reimar!

Reimar Döffinger wrote:
[...]
> Index: tests/faterun.sh
> ===================================================================
> --- tests/faterun.sh	(revision 34508)
> +++ tests/faterun.sh	(working copy)
> @@ -1,15 +1,20 @@
>  #!/bin/sh
> -i=$1
> -echo "running $i"
> -mkdir -p tests/res/$(dirname $i)
> -touch tests/res/$i.md5
> -./mplayer -noconfig all -lavdopts threads=4:bitexact -really-quiet -noconsolecontrols -nosound -benchmark -vo md5sum:outfile=tests/res/$i.md5 $FATE_SAMPLES/$i
> -ref_file=tests/ref/$i.md5
> +if [ -z "$FATE_SAMPLES" ] ; then

  I personally prefer using test instead of [ but that is not important.

> +  echo "FATE_SAMPLES is not set!"
> +  exit 1
> +fi
> +sample=$1

  The parameter $1 should be quoted with double quotes. Also see below.
Actually in this case it doesn't seem to be really needed.
 
> +md5out=tests/res/$sample.md5
> +ref_file=tests/ref/$sample.md5

  These too.

> +echo "running $sample"
> +mkdir -p $(dirname $md5out)

If I am not mistaken:
mkdir -p "$(dirname "$md5out")"

> +touch $md5out
> +./mplayer -noconfig all -lavdopts threads=4:bitexact -really-quiet -noconsolecontrols -nosound -benchmark -vo md5sum:outfile=$md5out $FATE_SAMPLES/$sample
>  if ! [ -e $ref_file ] ; then
>    touch tests/ref/empty.md5
>    ref_file=tests/ref/empty.md5
>  fi
> -if ! diff -uw $ref_file tests/res/$i.md5 ; then
> -  mv tests/res/$i.md5 tests/res/$i.md5.bad
> +if ! diff -uw $ref_file $md5out ; then
> +  mv $md5out $md5out.bad

  Here these variables should be in double quotes: ref_file and md5out,
FATE_SAMPLES, samples too. Ideally the vo md5sum outfile parameter
should contain an additional layer of quoting for the sub option parser.
Though that would need a bit more logic to work with all possible inputs.

  The importance of getting the quoting right here can be argued, but
I would rather like to see it done right. Not sure if the quoting stuff
can be security relevant in certain circumstances.
 
>    exit 1

  I would terminate with exit status 2 to differentiate between
usage error and other errors.

>  fi

  Well about the quoting remarks, consider to ignore them for now.
For the usual way this is invoked it should not be relevant as the
names should not contain whitespace that would lead to splitting.
The whitespace couldn't be easily properly handled in make and this
script usually gets invoked only from make.


  Alexander