diff mbox series

[ovs-dev] ovsdb-data: Don't put strings with digits in quotes.

Message ID 20190724165429.9060-1-i.maximets@samsung.com
State Changes Requested
Headers show
Series [ovs-dev] ovsdb-data: Don't put strings with digits in quotes. | expand

Commit Message

Ilya Maximets July 24, 2019, 4:54 p.m. UTC
No need to use quotes for strings like "br0".

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/ovsdb-data.c             |  2 +-
 tests/ovn-controller-vtep.at |  8 ++++----
 tests/ovn-nbctl.at           |  6 +++---
 tests/ovn-sbctl.at           | 20 ++++++++++----------
 tests/ovs-vsctl.at           | 32 ++++++++++++++++----------------
 tests/ovs-xapi-sync.at       |  8 ++++----
 tests/ovsdb-server.at        |  6 +++---
 tests/vtep-ctl.at            |  4 ++--
 8 files changed, 43 insertions(+), 43 deletions(-)

Comments

Ben Pfaff July 24, 2019, 5:08 p.m. UTC | #1
On Wed, Jul 24, 2019 at 07:54:29PM +0300, Ilya Maximets wrote:
> No need to use quotes for strings like "br0".
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>

One of the goals here is to quote anything that could be mistaken for
another type.  I think that it rejects digits because strings with
letters and digits could be UUIDs.  Maybe it should use a more specific
test for that.
Ilya Maximets July 24, 2019, 5:47 p.m. UTC | #2
On 24.07.2019 20:08, Ben Pfaff wrote:
> On Wed, Jul 24, 2019 at 07:54:29PM +0300, Ilya Maximets wrote:
>> No need to use quotes for strings like "br0".
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> 
> One of the goals here is to quote anything that could be mistaken for
> another type.  I think that it rejects digits because strings with
> letters and digits could be UUIDs.  Maybe it should use a more specific
> test for that.

Will this help:

diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index 3aa39cff8..c9b5f7610 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -674,6 +674,11 @@ string_needs_quotes(const char *s)
 {
     const char *p = s;
     unsigned char c;
+    struct uuid uuid;
+
+    if (uuid_from_string(&uuid, s)) {
+        return true;
+    }
 
     c = *p++;
     if (!isalpha(c) && c != '_') {
---

?

If so, I could update some tests and prepare v2 with this change tomorrow.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index b0fb20d31..3aa39cff8 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -681,7 +681,7 @@  string_needs_quotes(const char *s)
     }
 
     while ((c = *p++) != '\0') {
-        if (!isalpha(c) && c != '_' && c != '-' && c != '.') {
+        if (!isalpha(c) && !isdigit(c) && c != '_' && c != '-' && c != '.') {
             return true;
         }
     }
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
index 676cbe391..a3fe8cb88 100644
--- a/tests/ovn-controller-vtep.at
+++ b/tests/ovn-controller-vtep.at
@@ -136,28 +136,28 @@  AT_CHECK([ovn-sbctl --columns=ip list Encap | cut -d ':' -f2 | tr -d ' '], [0],
 AT_CHECK([vtep-ctl add-ls lswitch0 -- bind-ls br-vtep p0 100 lswitch0 -- bind-ls br-vtep p0 200 lswitch0 -- bind-ls br-vtep p1 300 lswitch0])
 OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Chassis | grep -- lswitch0`"])
 AT_CHECK([ovn-sbctl --columns=vtep_logical_switches list Chassis | cut -d ':' -f2 | tr -d ' ' ], [0], [dnl
-[["lswitch0"]]
+[[lswitch0]]
 ])
 
 # adds another logical switch and new vlan_bindings.
 AT_CHECK([vtep-ctl add-ls lswitch1 -- bind-ls br-vtep p0 300 lswitch1])
 OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Chassis | grep -- lswitch1`"])
 AT_CHECK([ovn-sbctl --columns=vtep_logical_switches list Chassis | cut -d ':' -f2 | tr -d ' '], [0], [dnl
-[["lswitch0","lswitch1"]]
+[[lswitch0,lswitch1]]
 ])
 
 # unbinds one port from lswitch0, nothing should change.
 AT_CHECK([vtep-ctl unbind-ls br-vtep p0 200])
 OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=vlan_bindings list physical_port p0 | grep -- '200='`"])
 AT_CHECK([ovn-sbctl --columns=vtep_logical_switches list Chassis | cut -d ':' -f2 | tr -d ' ' ], [0], [dnl
-[["lswitch0","lswitch1"]]
+[[lswitch0,lswitch1]]
 ])
 
 # unbinds all ports from lswitch0.
 AT_CHECK([vtep-ctl unbind-ls br-vtep p0 100 -- unbind-ls br-vtep p1 300])
 OVS_WAIT_UNTIL([test -z "`ovn-sbctl list Chassis | grep -- br-vtep_lswitch0`"])
 AT_CHECK([ovn-sbctl --columns=vtep_logical_switches list Chassis | cut -d ':' -f2 | tr -d ' ' ], [0], [dnl
-[["lswitch1"]]
+[[lswitch1]]
 ])
 
 # unbinds all ports from lswitch1.
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 18c5c1d42..d99d3af9b 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1594,7 +1594,7 @@  OVN_NBCTL_TEST([ovn_nbctl_port_groups], [port groups], [
 dnl Check that port group can be looked up by name
 AT_CHECK([ovn-nbctl create Port_Group name=pg0], [0], [ignore])
 AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl
-"pg0"
+pg0
 ])])
 
 OVN_NBCTL_TEST([ovn_nbctl_extra_newlines], [extra newlines], [
@@ -1603,10 +1603,10 @@  dnl daemon mode. All we have to do is ensure that each time we list database
 dnl information, there is not an extra newline at the beginning of the output.
 AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore])
 AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
-name                : "sw1"
+name                : sw1
 ])
 AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
-name                : "sw1"
+name                : sw1
 ])])
 
 OVN_NBCTL_TEST([ovn_nbctl_table_formatting], [table formatting], [
diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at
index 1266b56d0..650e357da 100644
--- a/tests/ovn-sbctl.at
+++ b/tests/ovn-sbctl.at
@@ -79,11 +79,11 @@  AT_CHECK([ovn-nbctl --wait=sb sync])
 AT_CHECK([ovn-sbctl lsp-bind vif0 ch0])
 
 AT_CHECK([ovn-sbctl show], [0], [dnl
-Chassis "ch0"
+Chassis ch0
     Encap stt
         ip: "1.2.3.5"
         options: {csum="true"}
-    Port_Binding "vif0"
+    Port_Binding vif0
 ])
 
 # adds another 'vif1'
@@ -92,12 +92,12 @@  AT_CHECK([ovn-nbctl lsp-set-addresses vif1 f0:ab:cd:ef:01:03])
 AT_CHECK([ovn-sbctl lsp-bind vif1 ch0])
 
 AT_CHECK([ovn-sbctl show | sed 's/vif[[0-9]]/vif/'], [0], [dnl
-Chassis "ch0"
+Chassis ch0
     Encap stt
         ip: "1.2.3.5"
         options: {csum="true"}
-    Port_Binding "vif"
-    Port_Binding "vif"
+    Port_Binding vif
+    Port_Binding vif
 ])
 
 # deletes 'vif1'
@@ -105,16 +105,16 @@  AT_CHECK([ovn-nbctl lsp-del vif1])
 AT_CHECK([ovn-nbctl --wait=sb sync])
 
 AT_CHECK([ovn-sbctl show], [0], [dnl
-Chassis "ch0"
+Chassis ch0
     Encap stt
         ip: "1.2.3.5"
         options: {csum="true"}
-    Port_Binding "vif0"
+    Port_Binding vif0
 ])
 
 uuid=$(ovn-sbctl --columns=_uuid list Chassis ch0 | cut -d ':' -f2 | tr -d ' ')
 AT_CHECK_UNQUOTED([ovn-sbctl --columns=logical_port,mac,chassis list Port_Binding], [0], [dnl
-logical_port        : "vif0"
+logical_port        : vif0
 mac                 : [["f0:ab:cd:ef:01:02"]]
 chassis             : ${uuid}
 ])
@@ -126,10 +126,10 @@  AT_CHECK([ovn-nbctl lsp-set-options vtep0 vtep_physical_switch=p0 vtep_logical_s
 
 AT_CHECK([ovn-sbctl --timeout=10 wait-until Port_Binding vtep0 options!={}])
 AT_CHECK([ovn-sbctl --columns=logical_port,mac,type,options list Port_Binding vtep0], [0], [dnl
-logical_port        : "vtep0"
+logical_port        : vtep0
 mac                 : [[]]
 type                : vtep
-options             : {vtep_logical_switch="l0", vtep_physical_switch="p0"}
+options             : {vtep_logical_switch=l0, vtep_physical_switch=p0}
 ])
 
 OVN_SBCTL_TEST_STOP
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 77604c58a..46fa3c5b1 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -453,9 +453,9 @@  value0
 key0=othervalue
 
 
-{"key1"="value1"}
-{"key2"="value2", "key3"="value3"}
-{"key4"="value4"}
+{key1=value1}
+{key2=value2, key3=value3}
+{key4=value4}
 ])
 AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
   [br-get-external-id a],
@@ -463,9 +463,9 @@  AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
   [get interface a2 external-ids],
   [get interface a3 external-ids])], [0],
 [
-{"key1"="value1"}
-{"key2"="value2", "key3"="value3"}
-{"key4"="value4"}
+{key1=value1}
+{key2=value2, key3=value3}
+{key4=value4}
 ])
 CHECK_BRIDGES([a, a, 0])
 CHECK_PORTS([a], [a1], [bond0])
@@ -731,7 +731,7 @@  flow_tables         : {}
 ipfix               : []
 mcast_snooping_enable: false
 mirrors             : []
-name                : "br0"
+name                : br0
 netflow             : []
 other_config        : {}
 ports               : []
@@ -747,14 +747,14 @@  AT_CHECK(
   [RUN_OVS_VSCTL([--columns=fail_mode,name,datapath_type list bridge])],
   [0],
   [[fail_mode           : []
-name                : "br0"
+name                : br0
 datapath_type       : ""
 ]], [ignore])
 AT_CHECK(
   [RUN_OVS_VSCTL([--columns=fail_mode,name,datapath_type find bridge])],
   [0],
   [[fail_mode           : []
-name                : "br0"
+name                : br0
 datapath_type       : ""
 ]], [ignore])
 AT_CHECK([
@@ -766,8 +766,8 @@  AT_CHECK(
   [RUN_OVS_VSCTL([--columns=name find bridge datapath_type!=foo])], [0], [stdout],
   [ignore])
 AT_CHECK([sed -n '/./p' stdout | sort], [0],
-  [[name                : "br0"
-name                : "br2"
+  [[name                : br0
+name                : br2
 ]])
 AT_CHECK(
   [RUN_OVS_VSCTL(
@@ -1142,9 +1142,9 @@  AT_CHECK(
 AT_CHECK(
   [sed -n -e '/uuid/p' -e '/name/p' -e '/mirrors/p' -e '/select/p' -e '/output/p' < stdout | uuidfilt], [0], [dnl
 [_uuid               : <0>
-name                : "eth0"
+name                : eth0
 _uuid               : <1>
-name                : "eth1"
+name                : eth1
 _uuid               : <2>
 name                : mymirror
 output_port         : <1>
@@ -1155,7 +1155,7 @@  select_src_port     : [<0>]
 select_vlan         : []
 _uuid               : <3>
 mirrors             : [<2>]
-name                : "br0"
+name                : br0
 ]])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
@@ -1214,7 +1214,7 @@  flow_tables         : {}
 ipfix               : []
 mcast_snooping_enable: false
 mirrors             : []
-name                : "br0"
+name                : br0
 netflow             : []
 other_config        : {}
 ports               : []
@@ -1357,7 +1357,7 @@  OVS_VSCTL_SETUP
 dnl First check that the database commands can refer to row by database UUID.
 AT_CHECK([RUN_OVS_VSCTL([add-br br0])])
 uuid=`[]RUN_OVS_VSCTL(get port br0 _uuid)`
-AT_CHECK([RUN_OVS_VSCTL([get port $uuid name])], [0], ["br0"
+AT_CHECK([RUN_OVS_VSCTL([get port $uuid name])], [0], [br0
 ])
 
 dnl Next check that, if a database row is given a name that has the same form
diff --git a/tests/ovs-xapi-sync.at b/tests/ovs-xapi-sync.at
index 2f00704ba..901bb9f6a 100644
--- a/tests/ovs-xapi-sync.at
+++ b/tests/ovs-xapi-sync.at
@@ -50,7 +50,7 @@  AT_CHECK([ovs_vsctl \
                 -- set Interface vif1.0 'external-ids={attached-mac="00:11:22:33:44:55", xs-network-uuid="9b66c68b-a74e-4d34-89a5-20a8ab352d1e", xs-vif-uuid="6ab1b260-398e-49ba-827b-c7696108964c", xs-vm-uuid="fcb8a3f6-dc04-41d2-8b8a-55afd2b755b8"'}])
 OVS_WAIT_UNTIL([ovs_vsctl get interface vif1.0 external-ids:iface-id >/dev/null 2>&1])
 AT_CHECK([ovs_vsctl get interface vif1.0 external-ids], [0],
-  [{attached-mac="00:11:22:33:44:55", iface-id="custom iface ID", iface-status=active, vm-id="custom vm ID", xs-network-uuid="9b66c68b-a74e-4d34-89a5-20a8ab352d1e", xs-vif-uuid="6ab1b260-398e-49ba-827b-c7696108964c", xs-vm-uuid="fcb8a3f6-dc04-41d2-8b8a-55afd2b755b8"}
+  [{attached-mac="00:11:22:33:44:55", iface-id="custom iface ID", iface-status=active, vm-id="custom vm ID", xs-network-uuid="9b66c68b-a74e-4d34-89a5-20a8ab352d1e", xs-vif-uuid="6ab1b260-398e-49ba-827b-c7696108964c", xs-vm-uuid=fcb8a3f6-dc04-41d2-8b8a-55afd2b755b8}
 ])
 
 # Add corresponding tap and check daemon's work.
@@ -59,15 +59,15 @@  OVS_WAIT_UNTIL([ovs_vsctl get interface tap1.0 external-ids:iface-id >/dev/null
 AT_CHECK([ovs_vsctl \
                 -- get interface vif1.0 external-ids \
                 -- get interface tap1.0 external-ids], [0],
-  [{attached-mac="00:11:22:33:44:55", iface-id="custom iface ID", iface-status=inactive, vm-id="custom vm ID", xs-network-uuid="9b66c68b-a74e-4d34-89a5-20a8ab352d1e", xs-vif-uuid="6ab1b260-398e-49ba-827b-c7696108964c", xs-vm-uuid="fcb8a3f6-dc04-41d2-8b8a-55afd2b755b8"}
-{attached-mac="00:11:22:33:44:55", iface-id="custom iface ID", iface-status=active, vm-id="custom vm ID", xs-network-uuid="9b66c68b-a74e-4d34-89a5-20a8ab352d1e", xs-vif-uuid="6ab1b260-398e-49ba-827b-c7696108964c", xs-vm-uuid="fcb8a3f6-dc04-41d2-8b8a-55afd2b755b8"}
+  [{attached-mac="00:11:22:33:44:55", iface-id="custom iface ID", iface-status=inactive, vm-id="custom vm ID", xs-network-uuid="9b66c68b-a74e-4d34-89a5-20a8ab352d1e", xs-vif-uuid="6ab1b260-398e-49ba-827b-c7696108964c", xs-vm-uuid=fcb8a3f6-dc04-41d2-8b8a-55afd2b755b8}
+{attached-mac="00:11:22:33:44:55", iface-id="custom iface ID", iface-status=active, vm-id="custom vm ID", xs-network-uuid="9b66c68b-a74e-4d34-89a5-20a8ab352d1e", xs-vif-uuid="6ab1b260-398e-49ba-827b-c7696108964c", xs-vm-uuid=fcb8a3f6-dc04-41d2-8b8a-55afd2b755b8}
 ])
 
 # Remove corresponding tap and check daemon's work.
 AT_CHECK([ovs_vsctl del-port tap1.0])
 OVS_WAIT_UNTIL([test `ovs_vsctl get interface vif1.0 external-ids:iface-status` = active])
 AT_CHECK([ovs_vsctl get interface vif1.0 external-ids], [0],
-  [{attached-mac="00:11:22:33:44:55", iface-id="custom iface ID", iface-status=active, vm-id="custom vm ID", xs-network-uuid="9b66c68b-a74e-4d34-89a5-20a8ab352d1e", xs-vif-uuid="6ab1b260-398e-49ba-827b-c7696108964c", xs-vm-uuid="fcb8a3f6-dc04-41d2-8b8a-55afd2b755b8"}
+  [{attached-mac="00:11:22:33:44:55", iface-id="custom iface ID", iface-status=active, vm-id="custom vm ID", xs-network-uuid="9b66c68b-a74e-4d34-89a5-20a8ab352d1e", xs-vif-uuid="6ab1b260-398e-49ba-827b-c7696108964c", xs-vm-uuid=fcb8a3f6-dc04-41d2-8b8a-55afd2b755b8}
 ])
 
 OVSDB_SERVER_SHUTDOWN
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 81f03d28b..09629410a 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -1203,8 +1203,8 @@  for i in `seq 1 $n_iterations`; do
     trigger_big_update $i
 done
 AT_CHECK([ovs-appctl -t ovsdb-client ovsdb-client/unblock])
-OVS_WAIT_UNTIL([grep "\"xyzzy$counter\"" ovsdb-client.out])
-OVS_WAIT_UNTIL([grep "\"xyzzy$counter\"" ovsdb-client-nonblock.out])
+OVS_WAIT_UNTIL([grep "xyzzy$counter" ovsdb-client.out])
+OVS_WAIT_UNTIL([grep "xyzzy$counter" ovsdb-client-nonblock.out])
 OVS_APP_EXIT_AND_WAIT([ovsdb-client])
 AT_CHECK([kill `cat nonblock.pid`])
 
@@ -1221,7 +1221,7 @@  echo "logged_updates=$logged_updates (expected less than $logged_nonblock_update
 AT_CHECK([test $logged_nonblock_updates -le $n_updates])
 AT_CHECK([test $logged_updates -lt $logged_nonblock_updates])
 AT_CHECK_UNQUOTED([ovs-vsctl get open_vswitch . system_version], [0],
-  ["xyzzy$counter"
+  [xyzzy$counter
 ])
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 AT_CLEANUP
diff --git a/tests/vtep-ctl.at b/tests/vtep-ctl.at
index e44eb33ee..f2b007f9a 100644
--- a/tests/vtep-ctl.at
+++ b/tests/vtep-ctl.at
@@ -939,9 +939,9 @@  AT_CHECK([vtep-ctl --timeout=5 -vreconnect:emer --db=unix:socket show | tail -n+
     Physical_Switch a
         management_ips: [["4.3.2.1"]]
         tunnel_ips: [["1.2.3.4"]]
-        Physical_Port "a1"
+        Physical_Port a1
             vlan_bindings:
-                100="ls1"
+                100=ls1
 ], [], [VTEP_CTL_CLEANUP])
 
 VTEP_CTL_CLEANUP