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

Mon Oct 30 12:51:46 UTC 2017

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

> index 50083a34d556..17682aa73db2 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -1656,26 +1656,26 @@ AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
>  00 00 00 07 00 00 00 00 00 0f 42 40 "
>   tail="00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00"
>  
> - echo -n "03 13 7f 90 00 00 00 02 00 03 00 00 00 00 00 00 "
> + printf "03 13 7f 90 00 00 00 02 00 03 00 00 00 00 00 00 "
>  
>   x=0
>   printf "%02x $pad7" $x
>   printf "%s$pad32" "classifier" | od -A n -t x1 -v -N 32 | tr '\n' ' '
> - echo -n "$mid 00 00 00 01  "
> - echo -n "00 00 00 00 00 01 23 76 00 00 00 00 00 01 9e 28 "
> + printf "$mid 00 00 00 01  "
> + printf "00 00 00 00 00 01 23 76 00 00 00 00 00 01 9e 28 "
>  
>   x=1
>   while test $x -lt 254; do
>     printf "%02x $pad7" $x
>     printf "%s$pad32" "table$x" | od -A n -t x1 -v -N 32 | tr '\n' ' '
> -   echo -n "$mid 00 00 00 00 $tail "
> +   printf "$mid 00 00 00 00 $tail "
>     x=`expr $x + 1`
>   done
>  
>   x=254
>   printf "%02x $pad7" $x
>   printf "%s$pad32" "table$x" | od -A n -t x1 -v -N 32 | tr '\n' ' '
> - echo -n "$mid 00 00 00 02 $tail") > in
> + printf "$mid 00 00 00 02 $tail") > in
>  AT_CHECK([ovs-ofctl ofp-print - < in], [0], [expout])
>  AT_CLEANUP
>  
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index c75a1acbf906..583907b9aa0d 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -7333,7 +7333,7 @@ m4_define([OFPROTO_DPIF_GET_FLOW],
>                         set Open_vSwitch . other_config:max-idle=10000], [], [],
>                         [m4_if([$1], [], [], [--dummy-numa="0,0,0,0,1,1,1,1"])])
>  
> -   func=`echo -n "$1_" | cut -c 4-`
> +   func=`printf '%s_' "$1" | cut -c 4-`
>     add_${func}of_ports br0 1 2
>  
>     AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> @@ -7665,7 +7665,7 @@ m4_define([OFPROTO_DPIF_MEGAFLOW_NORMAL],
>    [AT_SETUP([ofproto-dpif megaflow - normal$1])
>     OVS_VSWITCHD_START([], [], [], [m4_if([$1], [], [], [--dummy-numa="0,0,0,0,1,1,1,1"])])
>     AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
> -   func=`echo -n "$1_" | cut -c 4-`
> +   func=`printf '%s_' "$1" | cut -c 4-`
>     add_${func}of_ports br0 1 2
>     AT_CHECK([ovs-ofctl add-flow br0 action=normal])
>     AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> @@ -8068,7 +8068,7 @@ m4_define([OFPROTO_DPIF_MEGAFLOW_DISABLED],
>    [AT_SETUP([ofproto-dpif megaflow - disabled$1])
>     OVS_VSWITCHD_START([], [], [], [m4_if([$1], [], [], [--dummy-numa="0,0,0,0,1,1,1,1"])])
>     AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
> -   func=`echo -n "$1_" | cut -c 4-`
> +   func=`printf '%s_' "$1" | cut -c 4-`
>     add_${func}of_ports br0 1 2
>     AT_DATA([flows.txt], [dnl
>  table=0 in_port=1,ip,nw_dst=10.0.0.1 actions=output(2)
> diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
> index 7ae8cdb8a782..1632ad15da5a 100755
> --- a/tutorial/ovs-sandbox
> +++ b/tutorial/ovs-sandbox
> @@ -385,7 +385,7 @@ sleep 0.1
>  
>  #Wait for ovsdb-server to finish launching.
>  if test ! -e "$sandbox"/db.sock; then
> -    echo -n "Waiting for ovsdb-server to start..."
> +    printf "Waiting for ovsdb-server to start..."
>      while test ! -e "$sandbox"/db.sock; do
>          sleep 1;
>      done

-- 
Flavio