[cairo] [PATCH] Harden make-cairo-test-constructors.sh

[Bryce Harrington]

On Wed, Mar 11, 2015 at 03:29:26PM +0100, Andrea Canciani wrote:
> The make-cairo-test-constructors.sh script executes several commands
> without checking their success. This can lead to undetected errors,
> like those fixed in 86fad78fcd2bf987249890aea4eabcce02a58f45.
> 
> Setting the '-e' flag and writing to a temporary file ensures that the
> 'cairo-test-constructors.c' target completes succesfully only if no
> error occurred.
> ---
>  test/Makefile.am                     | 3 ++-
>  test/make-cairo-test-constructors.sh | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/test/Makefile.am b/test/Makefile.am
> index 950629b..a56e7ff 100644
> --- a/test/Makefile.am
> +++ b/test/Makefile.am
> @@ -81,7 +81,8 @@ noinst_SCRIPTS = check-refs.sh
>  TESTS += cairo-test-suite$(EXEEXT)
>  
>  cairo-test-constructors.c: Makefile $(test_sources) make-cairo-test-constructors.sh
> -	(cd $(srcdir) && sh ./make-cairo-test-constructors.sh $(test_sources)) > $@
> +	(cd $(srcdir) && sh ./make-cairo-test-constructors.sh $(test_sources)) > $@.tmp
> +	mv $@.tmp $@

This will properly avoid overwriting $@ if the script fails, and make
will bail at that point.  Does $@.tmp get created in that case?  If so,
does anything clean that stray file up?  Would mktemp be a cleaner
solution maybe?
  
>  cairo_test_suite_SOURCES = 		\
>  	$(cairo_test_suite_sources)	\
> diff --git a/test/make-cairo-test-constructors.sh b/test/make-cairo-test-constructors.sh
> index cb1391e..9b78eba 100644
> --- a/test/make-cairo-test-constructors.sh
> +++ b/test/make-cairo-test-constructors.sh
> @@ -1,5 +1,7 @@
>  #! /bin/sh
>  
> +set -e
> +

Can you verify that your sed exits with non-zero exit code in the
86fad78f case?

>  if test $# -eq 0; then
>      echo "$0: no input files." >&2
>      exit 0

Shouldn't this exit 1 since it's another error condition?

> -- 
> 1.9.5 (Apple Git-50.3)
> 
> -- 
> cairo mailing list
> cairo at cairographics.org
> http://lists.cairographics.org/mailman/listinfo/cairo