Skip to content

Commit

Permalink
treewide: Remove unnecessary strlen() calls.
Browse files Browse the repository at this point in the history
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>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
  • Loading branch information
igsilya authored and dceara committed Mar 3, 2023
1 parent c2f9682 commit a5238e6
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 24 deletions.
4 changes: 2 additions & 2 deletions controller/chassis.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_))) {
Expand Down
2 changes: 1 addition & 1 deletion controller/encaps.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion controller/physical.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
6 changes: 3 additions & 3 deletions controller/pinctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions ic/ovn-ic.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>");

Expand Down Expand Up @@ -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 "
Expand Down
5 changes: 2 additions & 3 deletions lib/ovn-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down
10 changes: 5 additions & 5 deletions northd/northd.c
Original file line number Diff line number Diff line change
Expand Up @@ -9624,7 +9624,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;
}

Expand Down Expand Up @@ -9699,7 +9699,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);
Expand Down Expand Up @@ -9990,7 +9990,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) {
Expand Down Expand Up @@ -10022,7 +10022,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) {
Expand Down Expand Up @@ -10338,7 +10338,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");
Expand Down
14 changes: 7 additions & 7 deletions utilities/ovn-nbctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit a5238e6

Please sign in to comment.