diff mbox series

[ovs-dev,1/2] chassis: Remove unreachable code

Message ID 20230125114552.49482-1-amusil@redhat.com
State Accepted
Headers show
Series [ovs-dev,1/2] chassis: Remove unreachable code | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Ales Musil Jan. 25, 2023, 11:45 a.m. UTC 
The parsing functions were always returning true so the
negative condition branch couldn't be ever reached.
The same applies for non-null ovnsb_idl_txn
inside "chassis_private_get_record" this is called under
condition that the ovnsb_idl_txn is not null thus
the inner condition is redundant.

Signed-off-by: Ales Musil <amusil@redhat.com>
---
 controller/chassis.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

Comments

Dumitru Ceara Feb. 15, 2023, 2:22 p.m. UTC | #1 
On 1/25/23 12:45, Ales Musil wrote:
> The parsing functions were always returning true so the
> negative condition branch couldn't be ever reached.
> The same applies for non-null ovnsb_idl_txn
> inside "chassis_private_get_record" this is called under
> condition that the ovnsb_idl_txn is not null thus
> the inner condition is redundant.
> 
> Signed-off-by: Ales Musil <amusil@redhat.com>
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara <dceara@redhat.com>
Dumitru Ceara March 8, 2023, 12:45 p.m. UTC | #2 
On 1/25/23 12:45, Ales Musil wrote:
> The parsing functions were always returning true so the
> negative condition branch couldn't be ever reached.
> The same applies for non-null ovnsb_idl_txn
> inside "chassis_private_get_record" this is called under
> condition that the ovnsb_idl_txn is not null thus
> the inner condition is redundant.
> 
> Signed-off-by: Ales Musil <amusil@redhat.com>
> ---

Applied to main, thanks!

Regards,
Dumitru
diff mbox series

Patch

diff --git a/controller/chassis.c b/controller/chassis.c
index acde00849..0f1e9e789 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -233,7 +233,7 @@  update_chassis_transport_zones(const struct sset *transport_zones,
  * Parse an ovs 'encap_type' string and stores the resulting types in the
  * 'encap_type_set' string set.
  */
-static bool
+static void
 chassis_parse_ovs_encap_type(const char *encap_type,
                              struct sset *encap_type_set)
 {
@@ -247,26 +247,23 @@  chassis_parse_ovs_encap_type(const char *encap_type,
             VLOG_INFO_RL(&rl, "Unknown tunnel type: %s", type);
         }
     }
-
-    return true;
 }
 
 /*
  * Parse an ovs 'encap_ip' string and stores the resulting IP representations
  * in the 'encap_ip_set' string set.
  */
-static bool
+static void
 chassis_parse_ovs_encap_ip(const char *encap_ip, struct sset *encap_ip_set)
 {
     sset_from_delimited_string(encap_ip_set, encap_ip, ",");
-    return true;
 }
 
 /*
  * Parse the ovs 'iface_types' and store them in the format required by the
  * Chassis record.
  */
-static bool
+static void
 chassis_parse_ovs_iface_types(char **iface_types, size_t n_iface_types,
                               struct ds *iface_types_str)
 {
@@ -274,7 +271,6 @@  chassis_parse_ovs_iface_types(char **iface_types, size_t n_iface_types,
         ds_put_format(iface_types_str, "%s,", iface_types[i]);
     }
     ds_chomp(iface_types_str, ',');
-    return true;
 }
 
 /*
@@ -329,26 +325,17 @@  chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table,
     ovs_cfg->trim_timeout_ms =
         get_trim_timeout(&cfg->external_ids, chassis_id);
 
-    if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) {
-        return false;
-    }
+    chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set);
 
     /* 'ovn-encap-ip' can accept a comma-delimited list of IP addresses instead
      * of a single IP address. Although this is undocumented, it can be used
      * to enable certain hardware-offloaded use cases in which a host has
      * multiple NICs and is assigning SR-IOV VFs to a guest (as logical ports).
      */
-    if (!chassis_parse_ovs_encap_ip(encap_ips, &ovs_cfg->encap_ip_set)) {
-        sset_destroy(&ovs_cfg->encap_type_set);
-        return false;
-    }
+    chassis_parse_ovs_encap_ip(encap_ips, &ovs_cfg->encap_ip_set);
 
-    if (!chassis_parse_ovs_iface_types(cfg->iface_types,
-                                       cfg->n_iface_types,
-                                       &ovs_cfg->iface_types)) {
-        sset_destroy(&ovs_cfg->encap_type_set);
-        sset_destroy(&ovs_cfg->encap_ip_set);
-    }
+    chassis_parse_ovs_iface_types(cfg->iface_types, cfg->n_iface_types,
+                                  &ovs_cfg->iface_types);
 
     ovs_cfg->is_interconn = get_is_interconn(&cfg->external_ids, chassis_id);
 
@@ -714,7 +701,7 @@  chassis_private_get_record(
             chassis_private_lookup_by_name(sbrec_chassis_pvt_by_name,
                                            chassis_id);
 
-    if (!chassis_p && ovnsb_idl_txn) {
+    if (!chassis_p) {
         return sbrec_chassis_private_insert(ovnsb_idl_txn);
     }