diff mbox

[ovs-dev,1/3] ovn.at: Use {} to make this less ambiguous

Message ID 1448530892-4893-1-git-send-email-yamamoto@midokura.com
State Accepted
Headers show

Commit Message

Takashi Yamamoto Nov. 26, 2015, 9:41 a.m. UTC
While (surprisingly to me) bash interprets $10 as ${1}0,
many other shells, including NetBSD's /bin/sh, interpret it as ${10}.

Signed-off-by: YAMAMOTO Takashi <yamamoto@midokura.com>
---
 tests/ovn.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Simon Horman Nov. 27, 2015, 2:16 a.m. UTC | #1
On Thu, Nov 26, 2015 at 06:41:30PM +0900, YAMAMOTO Takashi wrote:
> While (surprisingly to me) bash interprets $10 as ${1}0,
> many other shells, including NetBSD's /bin/sh, interpret it as ${10}.

The code-changes look fine to me but I wonder if the changelog could
be made a bit clearer. Something like:

ovn.at: Use {} to make variable expansion less ambiguous

While (surprisingly to me) bash interprets $10 as ${1}0,
many other shells, including NetBSD's /bin/sh, interpret it as ${10}.

Also use already assigned named variables rather than positional
parameters to make things a little more readable.

> 
> Signed-off-by: YAMAMOTO Takashi <yamamoto@midokura.com>
> ---
>  tests/ovn.at | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 68fcc9a..de0a830 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -964,7 +964,7 @@ done
>  test_ip() {
>      # This packet has bad checksums but logical L3 routing doesn't check.
>      local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5
> -    local packet=$3$208004500001c0000000040110000$4$50035111100080000
> +    local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}0035111100080000
>      shift; shift; shift; shift; shift
>      hv=hv`vif_to_hv $inport`
>      as $hv ovs-appctl netdev-dummy/receive vif$inport $packet
> -- 
> 2.4.9 (Apple Git-60)
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
Ben Pfaff Nov. 27, 2015, 6:17 p.m. UTC | #2
On Thu, Nov 26, 2015 at 06:41:30PM +0900, YAMAMOTO Takashi wrote:
> While (surprisingly to me) bash interprets $10 as ${1}0,
> many other shells, including NetBSD's /bin/sh, interpret it as ${10}.
> 
> Signed-off-by: YAMAMOTO Takashi <yamamoto@midokura.com>

Acked-by: Ben Pfaff <blp@ovn.org>

I guess that this is documented in the Autoconf manual, but I had never
really paid attention before:

'${10}'
     The 10th, 11th, ... positional parameters can be accessed only
     after a 'shift'.  The 7th Edition shell reported an error if given
     '${10}', and Solaris 10 '/bin/sh' still acts that way:

          $ set 1 2 3 4 5 6 7 8 9 10
          $ echo ${10}
          bad substitution

     Conversely, not all shells obey the Posix rule that when braces are
     omitted, multiple digits beyond a '$' imply the single-digit
     positional parameter expansion concatenated with the remaining
     literal digits.  To work around the issue, you must use braces.

          $ bash -c 'set a b c d e f g h i j; echo $10 ${1}0'
          a0 a0
          $ dash -c 'set a b c d e f g h i j; echo $10 ${1}0'
          j a0
Takashi Yamamoto Dec. 2, 2015, 6:54 a.m. UTC | #3
On Sat, Nov 28, 2015 at 3:17 AM, Ben Pfaff <blp@ovn.org> wrote:
> On Thu, Nov 26, 2015 at 06:41:30PM +0900, YAMAMOTO Takashi wrote:
>> While (surprisingly to me) bash interprets $10 as ${1}0,
>> many other shells, including NetBSD's /bin/sh, interpret it as ${10}.
>>
>> Signed-off-by: YAMAMOTO Takashi <yamamoto@midokura.com>
>
> Acked-by: Ben Pfaff <blp@ovn.org>

pushed with the commit message updated as Simon suggested.

>
> I guess that this is documented in the Autoconf manual, but I had never
> really paid attention before:

thank you.

>
> '${10}'
>      The 10th, 11th, ... positional parameters can be accessed only
>      after a 'shift'.  The 7th Edition shell reported an error if given
>      '${10}', and Solaris 10 '/bin/sh' still acts that way:
>
>           $ set 1 2 3 4 5 6 7 8 9 10
>           $ echo ${10}
>           bad substitution
>
>      Conversely, not all shells obey the Posix rule that when braces are
>      omitted, multiple digits beyond a '$' imply the single-digit
>      positional parameter expansion concatenated with the remaining
>      literal digits.  To work around the issue, you must use braces.
>
>           $ bash -c 'set a b c d e f g h i j; echo $10 ${1}0'
>           a0 a0
>           $ dash -c 'set a b c d e f g h i j; echo $10 ${1}0'
>           j a0
diff mbox

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index 68fcc9a..de0a830 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -964,7 +964,7 @@  done
 test_ip() {
     # This packet has bad checksums but logical L3 routing doesn't check.
     local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5
-    local packet=$3$208004500001c0000000040110000$4$50035111100080000
+    local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}0035111100080000
     shift; shift; shift; shift; shift
     hv=hv`vif_to_hv $inport`
     as $hv ovs-appctl netdev-dummy/receive vif$inport $packet