The ovs_assert() macro always evaluates its argument, even when NDEBUG is defined so that failure is ignored. This behavior wasn't documented, and thus a lot of code didn't rely on it. This commit documents the behavior and simplifies bits of code that heretofore didn't rely on it.
Signed-off-by: Ben Pfaff <[email protected]> --- include/openvswitch/util.h | 7 +++++-- lib/cmap.c | 6 ++---- lib/conntrack.c | 6 ++---- lib/hmapx.c | 6 ++---- lib/odp-execute.c | 5 +---- lib/odp-util.c | 5 +---- lib/ofp-msgs.c | 32 ++++++++------------------------ lib/ofp-util.c | 11 ++++------- lib/ovsdb-data.c | 4 +--- lib/shash.c | 3 +-- lib/sset.c | 6 ++---- ofproto/ofproto-dpif-xlate.c | 7 +++---- ovsdb/ovsdb-server.c | 4 +--- ovsdb/replication.c | 3 +-- 14 files changed, 34 insertions(+), 71 deletions(-) diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h index ad1b184f78d9..b4d49ee21804 100644 --- a/include/openvswitch/util.h +++ b/include/openvswitch/util.h @@ -44,8 +44,11 @@ const char *ovs_get_program_version(void); : (X) <= UINT_MAX / (Y) ? (unsigned int) (X) * (unsigned int) (Y) \ : UINT_MAX) -/* Like the standard assert macro, except writes the failure message to the - * log. */ +/* Like the standard assert macro, except: + * + * - Writes the failure message to the log. + * + * - Always evaluates the condition, even with NDEBUG. */ #ifndef NDEBUG #define ovs_assert(CONDITION) \ (OVS_LIKELY(CONDITION) \ diff --git a/lib/cmap.c b/lib/cmap.c index af29639b049c..07719a8fd286 100644 --- a/lib/cmap.c +++ b/lib/cmap.c @@ -843,11 +843,9 @@ cmap_replace(struct cmap *cmap, struct cmap_node *old_node, struct cmap_impl *impl = cmap_get_impl(cmap); uint32_t h1 = rehash(impl, hash); uint32_t h2 = other_hash(h1); - bool ok; - ok = cmap_replace__(impl, old_node, new_node, hash, h1) - || cmap_replace__(impl, old_node, new_node, hash, h2); - ovs_assert(ok); + ovs_assert(cmap_replace__(impl, old_node, new_node, hash, h1) || + cmap_replace__(impl, old_node, new_node, hash, h2)); if (!new_node) { impl->n--; diff --git a/lib/conntrack.c b/lib/conntrack.c index 562e767833d1..fe5fd0fe8be1 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -3080,11 +3080,9 @@ repl_ftp_v6_addr(struct dp_packet *pkt, struct ct_addr v6_addr_rep, return 0; } - const char *rc; char v6_addr_str[IPV6_SCAN_LEN] = {0}; - rc = inet_ntop(AF_INET6, &v6_addr_rep.ipv6_aligned, v6_addr_str, - IPV6_SCAN_LEN - 1); - ovs_assert(rc != NULL); + ovs_assert(inet_ntop(AF_INET6, &v6_addr_rep.ipv6_aligned, v6_addr_str, + IPV6_SCAN_LEN - 1)); size_t replace_addr_size = strlen(v6_addr_str); diff --git a/lib/hmapx.c b/lib/hmapx.c index 0440767c9c97..eadfe640ac11 100644 --- a/lib/hmapx.c +++ b/lib/hmapx.c @@ -116,8 +116,7 @@ hmapx_add(struct hmapx *map, void *data) void hmapx_add_assert(struct hmapx *map, void *data) { - bool added OVS_UNUSED = hmapx_add(map, data); - ovs_assert(added); + ovs_assert(hmapx_add(map, data)); } /* Removes all of the nodes from 'map'. */ @@ -156,8 +155,7 @@ hmapx_find_and_delete(struct hmapx *map, const void *data) void hmapx_find_and_delete_assert(struct hmapx *map, const void *data) { - bool deleted OVS_UNUSED = hmapx_find_and_delete(map, data); - ovs_assert(deleted); + ovs_assert(hmapx_find_and_delete(map, data)); } /* Searches for 'data' in 'map'. Returns its node, if found, otherwise a null diff --git a/lib/odp-execute.c b/lib/odp-execute.c index 20f33eb57acb..12bba83ea32c 100644 --- a/lib/odp-execute.c +++ b/lib/odp-execute.c @@ -198,10 +198,7 @@ odp_set_sctp(struct dp_packet *packet, const struct ovs_key_sctp *key, static void odp_set_tunnel_action(const struct nlattr *a, struct flow_tnl *tun_key) { - enum odp_key_fitness fitness; - - fitness = odp_tun_key_from_attr(a, tun_key); - ovs_assert(fitness != ODP_FIT_ERROR); + ovs_assert(odp_tun_key_from_attr(a, tun_key) != ODP_FIT_ERROR); } static void diff --git a/lib/odp-util.c b/lib/odp-util.c index 6a29a76de5cd..14d5af097ee4 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -6847,8 +6847,6 @@ commit_mpls_action(const struct flow *flow, struct flow *base, /* Otherwise, if there more LSEs in base than are common between * base and flow then pop the topmost one. */ ovs_be16 dl_type; - bool popped; - /* If all the LSEs are to be popped and this is not the outermost * LSE then use ETH_TYPE_MPLS as the ethertype parameter of the * POP_MPLS action instead of flow->dl_type. @@ -6865,8 +6863,7 @@ commit_mpls_action(const struct flow *flow, struct flow *base, dl_type = flow->dl_type; } nl_msg_put_be16(odp_actions, OVS_ACTION_ATTR_POP_MPLS, dl_type); - popped = flow_pop_mpls(base, base_n, flow->dl_type, NULL); - ovs_assert(popped); + ovs_assert(flow_pop_mpls(base, base_n, flow->dl_type, NULL)); base_n--; } } diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c index f9660fc81034..dd8894a524eb 100644 --- a/lib/ofp-msgs.c +++ b/lib/ofp-msgs.c @@ -313,8 +313,7 @@ static void ofphdrs_decode_assert(struct ofphdrs *hdrs, const struct ofp_header *oh, size_t length) { - enum ofperr error = ofphdrs_decode(hdrs, oh, length); - ovs_assert(!error); + ovs_assert(!ofphdrs_decode(hdrs, oh, length)); } static bool @@ -424,11 +423,8 @@ ofpraw_decode(enum ofpraw *raw, const struct ofp_header *oh) enum ofpraw ofpraw_decode_assert(const struct ofp_header *oh) { - enum ofperr error; enum ofpraw raw; - - error = ofpraw_decode(&raw, oh); - ovs_assert(!error); + ovs_assert(!ofpraw_decode(&raw, oh)); return raw; } @@ -524,11 +520,8 @@ ofpraw_pull(enum ofpraw *rawp, struct ofpbuf *msg) enum ofpraw ofpraw_pull_assert(struct ofpbuf *msg) { - enum ofperr error; enum ofpraw raw; - - error = ofpraw_pull(&raw, msg); - ovs_assert(!error); + ovs_assert(!ofpraw_pull(&raw, msg)); return raw; } @@ -633,11 +626,9 @@ ofpraw_alloc_stats_reply(const struct ofp_header *request, { enum ofpraw request_raw; enum ofpraw reply_raw; - enum ofperr error; - error = ofpraw_decode_partial(&request_raw, request, - ntohs(request->length)); - ovs_assert(!error); + ovs_assert(!ofpraw_decode_partial(&request_raw, request, + ntohs(request->length))); reply_raw = ofpraw_stats_request_to_reply(request_raw, request->version); ovs_assert(reply_raw); @@ -703,11 +694,9 @@ ofpraw_put_reply(enum ofpraw raw, const struct ofp_header *request, void ofpraw_put_stats_reply(const struct ofp_header *request, struct ofpbuf *buf) { - enum ofperr error; enum ofpraw raw; - error = ofpraw_decode_partial(&raw, request, ntohs(request->length)); - ovs_assert(!error); + ovs_assert(!ofpraw_decode_partial(&raw, request, ntohs(request->length))); raw = ofpraw_stats_request_to_reply(raw, request->version); ovs_assert(raw); @@ -801,7 +790,6 @@ ofpraw_stats_request_to_reply(enum ofpraw raw, uint8_t version) const struct raw_instance *instance = raw_instance_get(info, version); enum ofpraw reply_raw; struct ofphdrs hdrs; - enum ofperr error; hdrs = instance->hdrs; switch ((enum ofp_version)hdrs.version) { @@ -822,8 +810,7 @@ ofpraw_stats_request_to_reply(enum ofpraw raw, uint8_t version) OVS_NOT_REACHED(); } - error = ofpraw_from_ofphdrs(&reply_raw, &hdrs); - ovs_assert(!error); + ovs_assert(!ofpraw_from_ofphdrs(&reply_raw, &hdrs)); return reply_raw; } @@ -1016,11 +1003,8 @@ enum ofpraw ofpmp_decode_raw(struct ovs_list *replies) { struct ofpbuf *msg = ofpbuf_from_list(ovs_list_back(replies)); - enum ofperr error; enum ofpraw raw; - - error = ofpraw_decode_partial(&raw, msg->data, msg->size); - ovs_assert(!error); + ovs_assert(!ofpraw_decode_partial(&raw, msg->data, msg->size)); return raw; } diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 597112e4f84e..ed972f835cf9 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -2453,12 +2453,11 @@ ofputil_start_queue_get_config_reply(const struct ofp_header *request, struct ovs_list *replies) { struct ofpbuf *reply; - enum ofperr error; ofp_port_t port; uint32_t queue; - error = ofputil_decode_queue_get_config_request(request, &port, &queue); - ovs_assert(!error); + ovs_assert(!ofputil_decode_queue_get_config_request(request, &port, + &queue)); enum ofpraw raw = ofpraw_decode_assert(request); switch ((int) raw) { @@ -6227,8 +6226,7 @@ ofputil_decode_role_status(const struct ofp_header *oh, struct ofputil_role_status *rs) { struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); - enum ofpraw raw = ofpraw_pull_assert(&b); - ovs_assert(raw == OFPRAW_OFPT14_ROLE_STATUS); + ovs_assert(ofpraw_pull_assert(&b) == OFPRAW_OFPT14_ROLE_STATUS); const struct ofp14_role_status *r = b.msg; if (r->role != htonl(OFPCR12_ROLE_NOCHANGE) && @@ -6292,8 +6290,7 @@ ofputil_decode_requestforward(const struct ofp_header *outer, struct ofpbuf b = ofpbuf_const_initializer(outer, ntohs(outer->length)); /* Skip past outer message. */ - enum ofpraw outer_raw = ofpraw_pull_assert(&b); - ovs_assert(outer_raw == OFPRAW_OFPT14_REQUESTFORWARD); + ovs_assert(ofpraw_pull_assert(&b) == OFPRAW_OFPT14_REQUESTFORWARD); /* Validate inner message. */ if (b.size < sizeof(struct ofp_header)) { diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c index 87d8effd1d67..0c9945cd1318 100644 --- a/lib/ovsdb-data.c +++ b/lib/ovsdb-data.c @@ -1948,10 +1948,8 @@ ovsdb_datum_union(struct ovsdb_datum *a, const struct ovsdb_datum *b, } } if (n != a->n) { - struct ovsdb_error *error; a->n = n; - error = ovsdb_datum_sort(a, type->key.type); - ovs_assert(!error); + ovs_assert(!ovsdb_datum_sort(a, type->key.type)); } } diff --git a/lib/shash.c b/lib/shash.c index 3e94b172a65f..318a30ffc0f0 100644 --- a/lib/shash.c +++ b/lib/shash.c @@ -143,8 +143,7 @@ shash_add_once(struct shash *sh, const char *name, const void *data) void shash_add_assert(struct shash *sh, const char *name, const void *data) { - bool added OVS_UNUSED = shash_add_once(sh, name, data); - ovs_assert(added); + ovs_assert(shash_add_once(sh, name, data)); } /* Searches for 'name' in 'sh'. If it does not already exist, adds it along diff --git a/lib/sset.c b/lib/sset.c index be29cc71ef2e..3deb1f9de9be 100644 --- a/lib/sset.c +++ b/lib/sset.c @@ -163,8 +163,7 @@ sset_add_and_free(struct sset *set, char *name) void sset_add_assert(struct sset *set, const char *name) { - bool added OVS_UNUSED = sset_add(set, name); - ovs_assert(added); + ovs_assert(sset_add(set, name)); } /* Adds a copy of each of the 'n' names in 'names' to 'set'. */ @@ -214,8 +213,7 @@ sset_find_and_delete(struct sset *set, const char *name) void sset_find_and_delete_assert(struct sset *set, const char *name) { - bool deleted OVS_UNUSED = sset_find_and_delete(set, name); - ovs_assert(deleted); + ovs_assert(sset_find_and_delete(set, name)); } /* Removes a string from 'set' and returns a copy of it. The caller must free diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index d594ea40f7ee..315c67e71338 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1940,10 +1940,9 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle, int snaplen; /* Get the details of the mirror represented by the rightmost 1-bit. */ - bool has_mirror = mirror_get(xbridge->mbridge, raw_ctz(mirrors), - &vlans, &dup_mirrors, - &out, &snaplen, &out_vlan); - ovs_assert(has_mirror); + ovs_assert(mirror_get(xbridge->mbridge, raw_ctz(mirrors), + &vlans, &dup_mirrors, + &out, &snaplen, &out_vlan)); /* If this mirror selects on the basis of VLAN, and it does not select diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 7f2d19ef568b..3ac7bf472640 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -1291,11 +1291,9 @@ static void remove_db(struct server_config *config, struct shash_node *node) { struct db *db; - bool ok; db = node->data; - ok = ovsdb_jsonrpc_server_remove_db(config->jsonrpc, db->db); - ovs_assert(ok); + ovs_assert(ovsdb_jsonrpc_server_remove_db(config->jsonrpc, db->db)); close_db(db); shash_delete(config->all_dbs, node); diff --git a/ovsdb/replication.c b/ovsdb/replication.c index bac46c67f409..fc9ce6472725 100644 --- a/ovsdb/replication.c +++ b/ovsdb/replication.c @@ -118,10 +118,9 @@ replication_init(const char *sync_from_, const char *exclude_tables, { free(sync_from); sync_from = xstrdup(sync_from_); - char *err = set_blacklist_tables(exclude_tables, false); /* Caller should have verified that the 'exclude_tables' is * parseable. An error here is unexpected. */ - ovs_assert(!err); + ovs_assert(!set_blacklist_tables(exclude_tables, false)); replication_dbs_destroy(); -- 2.15.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
