diff mbox series

[ovs-dev,v2,1/3] treewide: Remove unnecessary strlen() calls.

Message ID 20230302131110.1665580-2-i.maximets@ovn.org
State Accepted
Headers show
Series northd: Optimize parsing of LSP addresses. | 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 success github build: passed

Commit Message

Ilya Maximets March 2, 2023, 1:11 p.m. UTC 
In many cases OVN components are using strlen() to check if the string
is not empty.  In that case, unnecessary scan of the whole string is
performed and the length is calculated, while it's enough to just check
the first byte.

Some functions also have completely unnecessary strlen() calls where
needed functionality can be achieved without them.

The most impactful changes in this commit are around extract_addresses()
function, and extract_lsp_addresses() in particular.  It is called by
northd for every logical port and involves 2 unnecessary strlen() calls.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 controller/chassis.c  |  4 ++--
 controller/encaps.c   |  2 +-
 controller/physical.c |  2 +-
 controller/pinctrl.c  |  6 +++---
 ic/ovn-ic.c           |  4 ++--
 lib/ovn-util.c        |  5 ++---
 northd/northd.c       | 10 +++++-----
 utilities/ovn-nbctl.c | 14 +++++++-------
 8 files changed, 23 insertions(+), 24 deletions(-)
diff mbox series

Patch

diff --git a/controller/chassis.c b/controller/chassis.c
index acde00849..0484e2fb9 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -99,9 +99,9 @@  static const char *
 get_hostname(const struct smap *ext_ids, const char *chassis_id)
 {
     const char *hostname = get_chassis_external_id_value(ext_ids, chassis_id,
-                                                         "hostname", "");
+                                                         "hostname", NULL);
 
-    if (strlen(hostname) == 0) {
+    if (!hostname) {
         static char hostname_[HOST_NAME_MAX + 1];
 
         if (gethostname(hostname_, sizeof(hostname_))) {
diff --git a/controller/encaps.c b/controller/encaps.c
index 5d383401d..2662eaf98 100644
--- a/controller/encaps.c
+++ b/controller/encaps.c
@@ -77,7 +77,7 @@  tunnel_create_name(struct tunnel_ctx *tc, const char *chassis_id)
     for (int i = 0; i < UINT16_MAX; i++) {
         const char *idx = get_chassis_idx(tc->ovs_table);
         char *port_name = xasprintf(
-            "ovn%s-%.*s-%x", idx, strlen(idx) ? 5 : 6, chassis_id, i);
+            "ovn%s-%.*s-%x", idx, idx[0] ? 5 : 6, chassis_id, i);
 
         if (!sset_contains(&tc->port_names, port_name)) {
             return port_name;
diff --git a/controller/physical.c b/controller/physical.c
index e718268ea..ec861f49c 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -505,7 +505,7 @@  populate_remote_chassis_macs(const struct sbrec_chassis *my_chassis,
         const char *tokens
             = get_chassis_mac_mappings(&chassis->other_config, chassis->name);
 
-        if (!strlen(tokens)) {
+        if (!tokens[0]) {
             continue;
         }
 
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 248b1a0e7..795847729 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -3775,12 +3775,12 @@  ipv6_ra_update_config(const struct sbrec_port_binding *pb)
 
     const char *dnssl = smap_get(&pb->options, "ipv6_ra_dnssl");
     if (dnssl) {
-        ds_put_buffer(&config->dnssl, dnssl, strlen(dnssl));
+        ds_put_cstr(&config->dnssl, dnssl);
     }
 
     const char *route_info = smap_get(&pb->options, "ipv6_ra_route_info");
     if (route_info) {
-        ds_put_buffer(&config->route_info, route_info, strlen(route_info));
+        ds_put_cstr(&config->route_info, route_info);
     }
 
     return config;
@@ -5825,7 +5825,7 @@  extract_addresses_with_port(const char *addresses,
     int ofs;
     if (!extract_addresses(addresses, laddrs, &ofs)) {
         return false;
-    } else if (ofs >= strlen(addresses)) {
+    } else if (!addresses[ofs]) {
         return true;
     }
 
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 9a80a7f68..1d0a062f6 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -1162,7 +1162,7 @@  add_static_to_routes_ad(
             ipv6_format_addr(&nexthop, &msg);
         }
 
-        ds_put_format(&msg, ", route_table: %s", strlen(nb_route->route_table)
+        ds_put_format(&msg, ", route_table: %s", nb_route->route_table[0]
                                                  ? nb_route->route_table
                                                  : "<main>");
 
@@ -1348,7 +1348,7 @@  sync_learned_routes(struct ic_context *ctx,
                 continue;
             }
 
-            if (strlen(isb_route->route_table) &&
+            if (isb_route->route_table[0] &&
                 strcmp(isb_route->route_table, ts_route_table)) {
                 if (VLOG_IS_DBG_ENABLED()) {
                     VLOG_DBG("Skip learning static route %s -> %s as either "
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index b12c027b9..47484ef01 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -128,7 +128,6 @@  parse_and_store_addresses(const char *address, struct lport_addresses *laddrs,
     const char *buf = address;
     const char *const start = buf;
     int buf_index = 0;
-    const char *buf_end = buf + strlen(address);
 
     if (extract_eth_addr) {
         if (!ovs_scan_len(buf, &buf_index, ETH_ADDR_SCAN_FMT,
@@ -151,7 +150,7 @@  parse_and_store_addresses(const char *address, struct lport_addresses *laddrs,
      * and store in the 'laddrs'. Break the loop if invalid data is found.
      */
     buf += buf_index;
-    while (buf < buf_end) {
+    while (*buf != '\0') {
         buf_index = 0;
         error = ip_parse_cidr_len(buf, &buf_index, &ip4, &plen);
         if (!error) {
@@ -205,7 +204,7 @@  extract_lsp_addresses(const char *address, struct lport_addresses *laddrs)
     int ofs;
     bool success = extract_addresses(address, laddrs, &ofs);
 
-    if (success && ofs < strlen(address)) {
+    if (success && address[ofs]) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
         VLOG_INFO_RL(&rl, "invalid syntax '%s' in address", address);
     }
diff --git a/northd/northd.c b/northd/northd.c
index 770a5b50e..6d37ea713 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -9623,7 +9623,7 @@  route_table_add(struct simap *route_tables, const char *route_table_name)
 static uint32_t
 get_route_table_id(struct simap *route_tables, const char *route_table_name)
 {
-    if (!route_table_name || !strlen(route_table_name)) {
+    if (!route_table_name || !route_table_name[0]) {
         return 0;
     }
 
@@ -9698,7 +9698,7 @@  parsed_routes_add(struct ovn_datapath *od, const struct hmap *ports,
     struct in6_addr nexthop;
     unsigned int plen;
     bool is_discard_route = !strcmp(route->nexthop, "discard");
-    bool valid_nexthop = strlen(route->nexthop) && !is_discard_route;
+    bool valid_nexthop = route->nexthop[0] && !is_discard_route;
     if (valid_nexthop) {
         if (!ip46_parse_cidr(route->nexthop, &nexthop, &plen)) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
@@ -9989,7 +9989,7 @@  find_static_route_outport(struct ovn_datapath *od, const struct hmap *ports,
                          route->output_port, route->ip_prefix);
             return false;
         }
-        if (strlen(route->nexthop)) {
+        if (route->nexthop[0]) {
             lrp_addr_s = find_lrp_member_ip(out_port, route->nexthop);
         }
         if (!lrp_addr_s) {
@@ -10021,7 +10021,7 @@  find_static_route_outport(struct ovn_datapath *od, const struct hmap *ports,
                 continue;
             }
 
-            if (strlen(route->nexthop)) {
+            if (route->nexthop[0]) {
                 lrp_addr_s = find_lrp_member_ip(out_port, route->nexthop);
             }
             if (lrp_addr_s) {
@@ -10337,7 +10337,7 @@  add_route(struct hmap *lflows, struct ovn_datapath *od,
     } else {
         ds_put_format(&common_actions, REG_ECMP_GROUP_ID" = 0; %s = ",
                       is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6);
-        if (gateway && strlen(gateway)) {
+        if (gateway && gateway[0]) {
             ds_put_cstr(&common_actions, gateway);
         } else {
             ds_put_format(&common_actions, "ip%s.dst", is_ipv4 ? "4" : "6");
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index ae4d6c403..45572fd30 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4553,7 +4553,7 @@  nbctl_lr_route_add(struct ctl_context *ctx)
     }
 
 cleanup:
-    if (next_hop && strlen(next_hop)) {
+    if (next_hop && next_hop[0]) {
         free(next_hop);
     }
     free(prefix);
@@ -6590,12 +6590,12 @@  print_route(const struct nbrec_logical_router_static_route *route,
 
     if (!strcmp(route->nexthop, "discard")) {
         next_hop = xasprintf("discard");
-    } else if (strlen(route->nexthop)) {
+    } else if (route->nexthop[0]) {
         next_hop = normalize_prefix_str(route->nexthop);
     }
     ds_put_format(s, "%25s %25s", prefix, next_hop);
     free(prefix);
-    if (strlen(next_hop)) {
+    if (next_hop[0]) {
         free(next_hop);
     }
 
@@ -6734,8 +6734,8 @@  nbctl_lr_route_list(struct ctl_context *ctx)
         if (!i || (i > 0 && strcmp(route->route_table,
                                    ipv4_routes[i - 1].route->route_table))) {
             ds_put_format(&ctx->output, "%sRoute Table %s:\n", i ? "\n" : "",
-                          strlen(route->route_table) ? route->route_table
-                                                     : "<main>");
+                          route->route_table[0] ? route->route_table
+                                                : "<main>");
         }
 
         print_route(ipv4_routes[i].route, &ctx->output, ecmp);
@@ -6760,8 +6760,8 @@  nbctl_lr_route_list(struct ctl_context *ctx)
         if (!i || (i > 0 && strcmp(route->route_table,
                                    ipv6_routes[i - 1].route->route_table))) {
             ds_put_format(&ctx->output, "%sRoute Table %s:\n", i ? "\n" : "",
-                          strlen(route->route_table) ? route->route_table
-                                                     : "<main>");
+                          route->route_table[0] ? route->route_table
+                                                : "<main>");
         }
 
         print_route(ipv6_routes[i].route, &ctx->output, ecmp);