[ovs-dev] [PATCH] treewide: Get rid of "echo -n", and add a test to prevent regression.

Ben Pfaff blp at ovn.org
Mon Oct 30 16:51:16 UTC 2017


On Mon, Oct 30, 2017 at 10:51:46AM -0200, Flavio Leitner wrote:
> On Fri, 27 Oct 2017 09:59:33 -0700
> Ben Pfaff <blp at ovn.org> wrote:
> 
> > "echo -n" is not POSIX and has spotty support in shells.
> > 
> > CC: Timothy Redaelli <tredaelli at redhat.com>
> > CC: Flavio Leitner <fbl at sysclose.org>
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> >  Makefile.am           | 11 +++++++++++
> >  tests/automake.mk     |  2 +-
> >  tests/ofp-print.at    | 10 +++++-----
> >  tests/ofproto-dpif.at |  6 +++---
> >  tutorial/ovs-sandbox  |  2 +-
> >  5 files changed, 21 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 31d6331792af..5ceb3822c976 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -303,6 +303,17 @@ check-endian:
> >  	fi
> >  .PHONY: check-endian
> >  
> > +ALL_LOCAL += check-echo-n
> > +check-echo-n:
> > +	@if test -e $(srcdir)/.git && (git --version) >/dev/null 2>&1 && \
> > +	  git --no-pager grep -n 'echo'' -n' $(srcdir); \
> > +	then \
> > +	  echo "See above for uses for \"echo"" -n\", which is non-POSIX"; \
> > +	  echo "and does not work with all shells.  Use \"printf\" instead."; \
> > +	  exit 1; \
> > +	fi
> > +.PHONY: check-echo-n
> > +
> >  ALL_LOCAL += thread-safety-check
> >  thread-safety-check:
> >  	@cd $(srcdir); \
> > diff --git a/tests/automake.mk b/tests/automake.mk
> > index 156b40f58d7c..198ba64e5cc2 100644
> > --- a/tests/automake.mk
> > +++ b/tests/automake.mk
> > @@ -201,7 +201,7 @@ $(valgrind_wrappers): tests/valgrind-wrapper.in
> >  CLEANFILES += $(valgrind_wrappers)
> >  EXTRA_DIST += tests/valgrind-wrapper.in
> >  
> > -VALGRIND = valgrind --log-file=valgrind.%p --leak-check=full \
> > +VALGRIND = valgrind --log-file=valgrind.%p --leak-check=no \
> >  	--suppressions=$(abs_top_srcdir)/tests/glibc.supp \
> >  	--suppressions=$(abs_top_srcdir)/tests/openssl.supp --num-callers=20
> >  HELGRIND = valgrind --log-file=helgrind.%p --tool=helgrind \
> > diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> 
> 
> The above change seems unrelated, otherwise the patch looks good to me.
> fbl

You are right.  I dropped this part of the patch.

Thanks for the review.  I applied this to master.


More information about the dev mailing list