Re: [PATCH] Harden make-cairo-test-constructors.sh
Bryce Harrington <[email protected]>
| Newsgroups | gmane.comp.lib.cairo |
|---|---|
| Message-ID | <[email protected]> |
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)) > [email protected] > + mv [email protected] $@ This will properly avoid overwriting $@ if the script fails, and make will bail at that point. Does [email protected] 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 > [email protected] > http://lists.cairographics.org/mailman/listinfo/cairo -- cairo mailing list [email protected] http://lists.cairographics.org/mailman/listinfo/cairo