On 3 Aug 2023, at 18:19, James Raphael Tiovalen wrote:
> This patch adds an assortment of `ovs_assert` statements to check for
> null pointers. We use assertions since it should be impossible for any
> of these pointers to be NULL.
>
> Signed-off-by: James Raphael Tiovalen <[email protected]>
> Reviewed-by: Simon Horman <[email protected]>
These changes look good to me, one request to Ilya to double-check the ovsdb
asserts.
Acked-by: Eelco Chaudron <[email protected]>
> ---
> lib/dp-packet.c | 1 +
> lib/odp-execute.c | 4 ++++
> lib/rtnetlink.c | 4 ++--
> lib/shash.c | 2 +-
> ovsdb/jsonrpc-server.c | 4 ++++
> ovsdb/monitor.c | 3 +++
> ovsdb/ovsdb-server.c | 2 ++
> ovsdb/transaction.c | 2 ++
> utilities/ovs-vsctl.c | 1 +
> vtep/vtep-ctl.c | 1 +
> 10 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index a9cf4bfc1..4a3e149e3 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -175,6 +175,7 @@ dp_packet_new_with_headroom(size_t size, size_t headroom)
> struct dp_packet *
> dp_packet_clone(const struct dp_packet *buffer)
> {
> + ovs_assert(buffer);
> return dp_packet_clone_with_headroom(buffer, 0);
> }
>
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 37f0f717a..eb03b57c4 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -147,6 +147,8 @@ odp_set_ipv4(struct dp_packet *packet, const struct
> ovs_key_ipv4 *key,
> uint8_t new_tos;
> uint8_t new_ttl;
>
> + ovs_assert(nh);
> +
> if (mask->ipv4_src) {
> ip_src_nh = get_16aligned_be32(&nh->ip_src);
> new_ip_src = key->ipv4_src | (ip_src_nh & ~mask->ipv4_src);
> @@ -287,6 +289,8 @@ set_arp(struct dp_packet *packet, const struct
> ovs_key_arp *key,
> {
> struct arp_eth_header *arp = dp_packet_l3(packet);
>
> + ovs_assert(arp);
> +
> if (!mask) {
> arp->ar_op = key->arp_op;
> arp->ar_sha = key->arp_sha;
> diff --git a/lib/rtnetlink.c b/lib/rtnetlink.c
> index f67352603..37078d00e 100644
> --- a/lib/rtnetlink.c
> +++ b/lib/rtnetlink.c
> @@ -112,7 +112,7 @@ rtnetlink_parse(struct ofpbuf *buf, struct
> rtnetlink_change *change)
> if (parsed) {
> const struct ifinfomsg *ifinfo;
>
> - ifinfo = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *ifinfo);
> + ifinfo = ofpbuf_at_assert(buf, NLMSG_HDRLEN, sizeof *ifinfo);
>
> /* Wireless events can be spammy and cause a
> * lot of unnecessary churn and CPU load in
> @@ -175,7 +175,7 @@ rtnetlink_parse(struct ofpbuf *buf, struct
> rtnetlink_change *change)
> if (parsed) {
> const struct ifaddrmsg *ifaddr;
>
> - ifaddr = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *ifaddr);
> + ifaddr = ofpbuf_at_assert(buf, NLMSG_HDRLEN, sizeof *ifaddr);
>
> change->nlmsg_type = nlmsg->nlmsg_type;
> change->if_index = ifaddr->ifa_index;
> diff --git a/lib/shash.c b/lib/shash.c
> index b62b586fc..92260cddf 100644
> --- a/lib/shash.c
> +++ b/lib/shash.c
> @@ -269,7 +269,7 @@ void *
> shash_find_and_delete_assert(struct shash *sh, const char *name)
> {
> void *data = shash_find_and_delete(sh, name);
> - ovs_assert(data != NULL);
> + ovs_assert(data);
> return data;
> }
>
> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> index 5361b3c76..a3ca48a7b 100644
> --- a/ovsdb/jsonrpc-server.c
> +++ b/ovsdb/jsonrpc-server.c
> @@ -1131,6 +1131,8 @@ static void
> ovsdb_jsonrpc_trigger_create(struct ovsdb_jsonrpc_session *s, struct ovsdb
> *db,
> struct jsonrpc_msg *request)
> {
> + ovs_assert(db);
> +
> /* Check for duplicate ID. */
> size_t hash = json_hash(request->id, 0);
> struct ovsdb_jsonrpc_trigger *t
> @@ -1391,6 +1393,8 @@ ovsdb_jsonrpc_monitor_create(struct
> ovsdb_jsonrpc_session *s, struct ovsdb *db,
> enum ovsdb_monitor_version version,
> const struct json *request_id)
> {
> + ovs_assert(db);
> +
> struct ovsdb_jsonrpc_monitor *m = NULL;
> struct ovsdb_monitor *dbmon = NULL;
> struct json *monitor_id, *monitor_requests;
> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> index af88b96e3..ca09780a9 100644
> --- a/ovsdb/monitor.c
> +++ b/ovsdb/monitor.c
> @@ -1329,6 +1329,7 @@ ovsdb_monitor_table_add_select(struct ovsdb_monitor
> *dbmon,
> struct ovsdb_monitor_table * mt;
>
> mt = shash_find_data(&dbmon->tables, table->schema->name);
> + ovs_assert(mt);
> mt->select |= select;
> }
>
> @@ -1713,6 +1714,8 @@ ovsdb_monitor_hash(const struct ovsdb_monitor *dbmon,
> size_t basis)
> for (i = 0; i < n; i++) {
> struct ovsdb_monitor_table *mt = nodes[i]->data;
>
> + ovs_assert(mt);
> +
> basis = hash_pointer(mt->table, basis);
> basis = hash_3words(mt->select, mt->n_columns, basis);
>
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index 8e623118b..1728d3a24 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -2257,6 +2257,8 @@ save_config(struct server_config *config)
> static void
> sset_from_json(struct sset *sset, const struct json *array)
> {
> + ovs_assert(array);
> +
> size_t i;
>
> sset_clear(sset);
> diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
> index 7cf4a851a..4fdc5bcea 100644
> --- a/ovsdb/transaction.c
> +++ b/ovsdb/transaction.c
> @@ -34,6 +34,7 @@
> #include "storage.h"
> #include "table.h"
> #include "uuid.h"
> +#include "util.h"
>
> VLOG_DEFINE_THIS_MODULE(transaction);
>
> @@ -576,6 +577,7 @@ ovsdb_txn_update_weak_refs(struct ovsdb_txn *txn
> OVS_UNUSED,
> dst_row = CONST_CAST(struct ovsdb_row *,
> ovsdb_table_get_row(weak->dst_table, &weak->dst));
>
> + ovs_assert(dst_row);
> ovs_assert(!ovsdb_row_find_weak_ref(dst_row, weak));
> hmap_insert(&dst_row->dst_refs, &weak->dst_node,
> ovsdb_weak_ref_hash(weak));
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index f55c2965a..b06328f50 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -817,6 +817,7 @@ vsctl_context_populate_cache(struct ctl_context *ctx)
> continue;
> }
> br = shash_find_data(&vsctl_ctx->bridges, br_cfg->name);
> + ovs_assert(br);
> for (j = 0; j < br_cfg->n_ports; j++) {
> struct ovsrec_port *port_cfg = br_cfg->ports[j];
> struct vsctl_port *port;
> diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
> index bb22cc5ce..ff3ed14dc 100644
> --- a/vtep/vtep-ctl.c
> +++ b/vtep/vtep-ctl.c
> @@ -1065,6 +1065,7 @@ vtep_ctl_context_populate_cache(struct ctl_context *ctx)
> continue;
> }
> ps = shash_find_data(&vtepctl_ctx->pswitches, ps_cfg->name);
> + ovs_assert(ps);
> for (j = 0; j < ps_cfg->n_ports; j++) {
> struct vteprec_physical_port *port_cfg = ps_cfg->ports[j];
> struct vtep_ctl_port *port;
> --
> 2.41.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
