[ovs-dev] [PATCH 2/7] ovs-vsctl: Fix missing ovs_scan() return checks.

[Eelco Chaudron]

Coverity flagged some cases where ovs_scan() was used without
checking its return value. This patch addresses the issue by
adding proper return checks and error reporting.

Additionally, test cases were added to ensure correctness.

Signed-off-by: Eelco Chaudron <echau...@redhat.com>
---
 tests/ovs-vsctl.at    | 15 +++++++++++++++
 utilities/ovs-vsctl.c | 25 ++++++++++++++++++++-----
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index a0e49155a..0778c4480 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1178,9 +1178,15 @@ AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 
icmp_first=2 icmp_reply=3])],
 ])
 AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout 
Policies: icmp_first=2 icmp_reply=3
 ])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=UNKNOWN icmp_first=2 
icmp_reply=3])],
+  [1], [], [ovs-vsctl: invalid zone argument, zone=UNKNOWN
+])
 AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=11])],
   [1], [], [ovs-vsctl: zone id 11 does not have a policy
 ])
+AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=UNKNOWN])],
+  [1], [], [ovs-vsctl: invalid zone argument, zone=UNKNOWN
+])
 AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout 
Policies: icmp_first=2 icmp_reply=3
 ])
 
@@ -1206,6 +1212,15 @@ AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default 
-1])],
 AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev default])],
   [1], [], [ovs-vsctl: datapath netdev does not have a limit
 ])
+AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev UNKNOWN 1])],
+  [1], [], [ovs-vsctl: invalid zone id, UNKNOWN
+])
+AT_CHECK([RUN_OVS_VSCTL([set-zone-limit netdev default VALUE])],
+  [1], [], [ovs-vsctl: invalid limit, VALUE
+])
+AT_CHECK([RUN_OVS_VSCTL([del-zone-limit netdev UNKNOWN])],
+  [1], [], [ovs-vsctl: invalid zone id, UNKNOWN
+])
 
 AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 
'capabilities={recirc=true}' -- set Open_vSwitch . datapaths:"system"=@m])], 
[0], [stdout])
 AT_CHECK([RUN_OVS_VSCTL([list-dp-cap nosystem])],
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index dd494622f..691b974e3 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1294,9 +1294,12 @@ cmd_add_zone_tp(struct ctl_context *ctx)
     int64_t zone_id;
 
     const char *dp_name = ctx->argv[1];
-    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
     bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
 
+    if (!ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id)) {
+        ctl_fatal("invalid zone argument, %s", ctx->argv[2]);
+    }
+
     struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name);
     if (!dp) {
         ctl_fatal("datapath %s does not exist", dp_name);
@@ -1331,7 +1334,10 @@ cmd_del_zone_tp(struct ctl_context *ctx)
 
     bool must_exist = !shash_find(&ctx->options, "--if-exists");
     const char *dp_name = ctx->argv[1];
-    ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id);
+
+    if (!ovs_scan(ctx->argv[2], "zone=%"SCNi64, &zone_id)) {
+        ctl_fatal("invalid zone argument, %s", ctx->argv[2]);
+    }
 
     struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name);
     if (!dp) {
@@ -1396,8 +1402,14 @@ cmd_set_zone_limit(struct ctl_context *ctx)
 
     const char *dp_name = ctx->argv[1];
 
-    ovs_scan(ctx->argv[2], "%"SCNi64, &zone_id);
-    ovs_scan(ctx->argv[3], "%"SCNi64, &limit);
+    if (!ovs_scan(ctx->argv[2], "%"SCNi64, &zone_id)
+        && strcmp(ctx->argv[2], "default")) {
+        ctl_fatal("invalid zone id, %s", ctx->argv[2]);
+    }
+
+    if (!ovs_scan(ctx->argv[3], "%"SCNi64, &limit)) {
+        ctl_fatal("invalid limit, %s", ctx->argv[3]);
+    }
 
     struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name);
     if (!dp) {
@@ -1435,7 +1447,10 @@ cmd_del_zone_limit(struct ctl_context *ctx)
     bool must_exist = !shash_find(&ctx->options, "--if-exists");
     const char *dp_name = ctx->argv[1];
 
-    ovs_scan(ctx->argv[2], "%"SCNi64, &zone_id);
+    if (!ovs_scan(ctx->argv[2], "%"SCNi64, &zone_id)
+        && strcmp(ctx->argv[2], "default")) {
+        ctl_fatal("invalid zone id, %s", ctx->argv[2]);
+    }
 
     struct ovsrec_datapath *dp = find_datapath(vsctl_ctx, dp_name);
     if (!dp) {
-- 
2.47.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev