On 16 Mar 2023, at 17:36, James Raphael Tiovalen wrote:
> This commit addresses several high and medium-impact Coverity defects by
> fixing several possible null-pointer dereferences and potentially
> uninitialized variables.
>
> There were cases when crashes were encountered when some null pointers
> were dereferenced. Null pointer checks and alternative code paths have
> been added to prevent unwanted crashes. Additionally, some struct
> variables were not initialized. Zero-initializations have been added to
> prevent potential data leaks or undefined behavior.
>
> Some variables would always be initialized by either the code flow or
> the compiler and some pointers would never be null. Thus, some Coverity
> reports might be false positives. That said, it is still considered best
> practice to perform null pointer checks and initialize variables upfront
> just in case to ensure the overall resilience and security of OVS, as
> long as they do not impact performance-critical code. As a bonus, it
> would also make static analyzer tools, such as Coverity, happy.
>
> Unit tests have been successfully executed via `make check` and they
> successfully passed.
I did not review this, but I noticed you do null checks for memset() and
memcpy(). But there are functions for this in OVS like nullable_memset() and
nullable_memcpy(), maybe some of the changes might benefit from using this.
In addition, there is also a nullable_string_is_equal() which might be useful
in some cases.
//Eelco
> Signed-off-by: James Raphael Tiovalen <[email protected]>
> ---
> v5:
> Improve commit message to better describe the intent of this patch.
>
> v4:
> - Fix some apply-robot checkpatch errors and warnings.
> - Fix github-robot build failure: linux clang test asan.
>
> v3:
> Fix some apply-robot checkpatch errors and warnings.
>
> v2:
> Fix some apply-robot checkpatch errors and warnings.
> ---
> lib/dp-packet.c | 69 ++++++++++++------
> lib/dpctl.c | 66 +++++++++--------
> lib/json.c | 32 +++++---
> lib/latch-unix.c | 2 +-
> lib/memory.c | 10 ++-
> lib/netdev-native-tnl.c | 48 +++++++-----
> lib/odp-execute.c | 91 ++++++++++++-----------
> lib/pcap-file.c | 5 +-
> lib/rtnetlink.c | 11 ++-
> lib/sflow_agent.c | 91 +++++++++++++++--------
> lib/shash.c | 4 +-
> lib/sset.c | 7 +-
> ovsdb/condition.c | 10 ++-
> ovsdb/file.c | 26 +++++--
> ovsdb/jsonrpc-server.c | 5 +-
> ovsdb/monitor.c | 49 ++++++++++---
> ovsdb/ovsdb-client.c | 45 ++++++++----
> ovsdb/ovsdb-server.c | 15 ++--
> ovsdb/ovsdb-util.c | 9 +++
> ovsdb/ovsdb.c | 124 ++++++++++++++++---------------
> ovsdb/query.c | 4 +-
> ovsdb/replication.c | 14 ++--
> ovsdb/row.c | 4 +-
> ovsdb/table.c | 2 +-
> ovsdb/transaction.c | 8 +-
> utilities/ovs-vsctl.c | 157 ++++++++++++++++++++++++----------------
> vtep/vtep-ctl.c | 79 +++++++++++---------
> 27 files changed, 607 insertions(+), 380 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index ae8ab5800..50cb30681 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -171,7 +171,11 @@ dp_packet_new_with_headroom(size_t size, size_t headroom)
> struct dp_packet *
> dp_packet_clone(const struct dp_packet *buffer)
> {
> - return dp_packet_clone_with_headroom(buffer, 0);
> + if (buffer) {
> + return dp_packet_clone_with_headroom(buffer, 0);
> + } else {
> + return NULL;
> + }
> }
>
> /* Creates and returns a new dp_packet whose data are copied from 'buffer'.
> @@ -183,26 +187,32 @@ dp_packet_clone_with_headroom(const struct dp_packet
> *buffer, size_t headroom)
> struct dp_packet *new_buffer;
> uint32_t mark;
>
> - new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer),
> - dp_packet_size(buffer),
> - headroom);
> - /* Copy the following fields into the returned buffer: l2_pad_size,
> - * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */
> - memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size,
> - sizeof(struct dp_packet) -
> - offsetof(struct dp_packet, l2_pad_size));
> + const void *data_dp = dp_packet_data(buffer);
>
> - *dp_packet_ol_flags_ptr(new_buffer) = *dp_packet_ol_flags_ptr(buffer);
> - *dp_packet_ol_flags_ptr(new_buffer) &= DP_PACKET_OL_SUPPORTED_MASK;
> + if (data_dp) {
> + new_buffer = dp_packet_clone_data_with_headroom(data_dp,
> +
> dp_packet_size(buffer),
> + headroom);
> + /* Copy the following fields into the returned buffer: l2_pad_size,
> + * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */
> + memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size,
> + sizeof(struct dp_packet) -
> + offsetof(struct dp_packet, l2_pad_size));
>
> - if (dp_packet_rss_valid(buffer)) {
> - dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer));
> - }
> - if (dp_packet_has_flow_mark(buffer, &mark)) {
> - dp_packet_set_flow_mark(new_buffer, mark);
> - }
> + *dp_packet_ol_flags_ptr(new_buffer) =
> *dp_packet_ol_flags_ptr(buffer);
> + *dp_packet_ol_flags_ptr(new_buffer) &= DP_PACKET_OL_SUPPORTED_MASK;
> +
> + if (dp_packet_rss_valid(buffer)) {
> + dp_packet_set_rss_hash(new_buffer,
> dp_packet_get_rss_hash(buffer));
> + }
> + if (dp_packet_has_flow_mark(buffer, &mark)) {
> + dp_packet_set_flow_mark(new_buffer, mark);
> + }
>
> - return new_buffer;
> + return new_buffer;
> + } else {
> + return NULL;
> + }
> }
>
> /* Creates and returns a new dp_packet that initially contains a copy of the
> @@ -323,8 +333,11 @@ dp_packet_shift(struct dp_packet *b, int delta)
>
> if (delta != 0) {
> char *dst = (char *) dp_packet_data(b) + delta;
> - memmove(dst, dp_packet_data(b), dp_packet_size(b));
> - dp_packet_set_data(b, dst);
> + const void *data_dp = dp_packet_data(b);
> + if (data_dp) {
> + memmove(dst, data_dp, dp_packet_size(b));
> + dp_packet_set_data(b, dst);
> + }
> }
> }
>
> @@ -348,7 +361,9 @@ void *
> dp_packet_put_zeros(struct dp_packet *b, size_t size)
> {
> void *dst = dp_packet_put_uninit(b, size);
> - memset(dst, 0, size);
> + if (dst) {
> + memset(dst, 0, size);
> + }
> return dst;
> }
>
> @@ -359,7 +374,9 @@ void *
> dp_packet_put(struct dp_packet *b, const void *p, size_t size)
> {
> void *dst = dp_packet_put_uninit(b, size);
> - memcpy(dst, p, size);
> + if (dst) {
> + memcpy(dst, p, size);
> + }
> return dst;
> }
>
> @@ -431,7 +448,9 @@ void *
> dp_packet_push_zeros(struct dp_packet *b, size_t size)
> {
> void *dst = dp_packet_push_uninit(b, size);
> - memset(dst, 0, size);
> + if (dst) {
> + memset(dst, 0, size);
> + }
> return dst;
> }
>
> @@ -442,7 +461,9 @@ void *
> dp_packet_push(struct dp_packet *b, const void *p, size_t size)
> {
> void *dst = dp_packet_push_uninit(b, size);
> - memcpy(dst, p, size);
> + if (dst) {
> + memcpy(dst, p, size);
> + }
> return dst;
> }
>
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index c501a0cd7..6472990cc 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -336,12 +336,14 @@ dpctl_add_if(int argc OVS_UNUSED, const char *argv[],
> value = "";
> }
>
> - if (!strcmp(key, "type")) {
> - type = value;
> - } else if (!strcmp(key, "port_no")) {
> - port_no = u32_to_odp(atoi(value));
> - } else if (!smap_add_once(&args, key, value)) {
> - dpctl_error(dpctl_p, 0, "duplicate \"%s\" option", key);
> + if (key) {
> + if (!strcmp(key, "type")) {
> + type = value;
> + } else if (!strcmp(key, "port_no")) {
> + port_no = u32_to_odp(atoi(value));
> + } else if (!smap_add_once(&args, key, value)) {
> + dpctl_error(dpctl_p, 0, "duplicate \"%s\" option", key);
> + }
> }
> }
>
> @@ -454,25 +456,29 @@ dpctl_set_if(int argc, const char *argv[], struct
> dpctl_params *dpctl_p)
> value = "";
> }
>
> - if (!strcmp(key, "type")) {
> - if (strcmp(value, type)) {
> - dpctl_error(dpctl_p, 0,
> - "%s: can't change type from %s to %s",
> - name, type, value);
> - error = EINVAL;
> - goto next_destroy_args;
> - }
> - } else if (!strcmp(key, "port_no")) {
> - if (port_no != u32_to_odp(atoi(value))) {
> - dpctl_error(dpctl_p, 0, "%s: can't change port number
> from"
> - " %"PRIu32" to %d", name, port_no,
> atoi(value));
> - error = EINVAL;
> - goto next_destroy_args;
> + if (key) {
> + if (!strcmp(key, "type")) {
> + if (strcmp(value, type)) {
> + dpctl_error(dpctl_p, 0,
> + "%s: can't change type from %s to %s",
> + name, type, value);
> + error = EINVAL;
> + goto next_destroy_args;
> + }
> + } else if (!strcmp(key, "port_no")) {
> + if (port_no != u32_to_odp(atoi(value))) {
> + dpctl_error(dpctl_p, 0,
> + "%s: can't change port number from"
> + " %"PRIu32" to %d", name, port_no,
> + atoi(value));
> + error = EINVAL;
> + goto next_destroy_args;
> + }
> + } else if (value[0] == '\0') {
> + smap_remove(&args, key);
> + } else {
> + smap_replace(&args, key, value);
> }
> - } else if (value[0] == '\0') {
> - smap_remove(&args, key);
> - } else {
> - smap_replace(&args, key, value);
> }
> }
>
> @@ -693,12 +699,14 @@ show_dpif(struct dpif *dpif, struct dpctl_params
> *dpctl_p)
> error = netdev_get_config(netdev, &config);
> if (!error) {
> const struct smap_node **nodes = smap_sort(&config);
> - for (size_t j = 0; j < smap_count(&config); j++) {
> - const struct smap_node *node = nodes[j];
> - dpctl_print(dpctl_p, "%c %s=%s", j ? ',' : ':',
> - node->key, node->value);
> + if (nodes) {
> + for (size_t j = 0; j < smap_count(&config); j++) {
> + const struct smap_node *node = nodes[j];
> + dpctl_print(dpctl_p, "%c %s=%s", j ? ',' : ':',
> + node->key, node->value);
> + }
> + free(nodes);
> }
> - free(nodes);
> } else {
> dpctl_print(dpctl_p, ", could not retrieve configuration
> "
> "(%s)", ovs_strerror(error));
> diff --git a/lib/json.c b/lib/json.c
> index aded8bb01..09683b54f 100644
> --- a/lib/json.c
> +++ b/lib/json.c
> @@ -342,8 +342,12 @@ json_object(const struct json *json)
> bool
> json_boolean(const struct json *json)
> {
> - ovs_assert(json->type == JSON_TRUE || json->type == JSON_FALSE);
> - return json->type == JSON_TRUE;
> + if (json) {
> + ovs_assert(json->type == JSON_TRUE || json->type == JSON_FALSE);
> + return json->type == JSON_TRUE;
> + } else {
> + return false;
> + }
> }
>
> double
> @@ -497,13 +501,15 @@ json_hash_object(const struct shash *object, size_t
> basis)
> size_t n, i;
>
> nodes = shash_sort(object);
> - n = shash_count(object);
> - for (i = 0; i < n; i++) {
> - const struct shash_node *node = nodes[i];
> - basis = hash_string(node->name, basis);
> - basis = json_hash(node->data, basis);
> + if (nodes) {
> + n = shash_count(object);
> + for (i = 0; i < n; i++) {
> + const struct shash_node *node = nodes[i];
> + basis = hash_string(node->name, basis);
> + basis = json_hash(node->data, basis);
> + }
> + free(nodes);
> }
> - free(nodes);
> return basis;
> }
>
> @@ -1654,11 +1660,13 @@ json_serialize_object(const struct shash *object,
> struct json_serializer *s)
> size_t n, i;
>
> nodes = shash_sort(object);
> - n = shash_count(object);
> - for (i = 0; i < n; i++) {
> - json_serialize_object_member(i, nodes[i], s);
> + if (nodes) {
> + n = shash_count(object);
> + for (i = 0; i < n; i++) {
> + json_serialize_object_member(i, nodes[i], s);
> + }
> + free(nodes);
> }
> - free(nodes);
> } else {
> struct shash_node *node;
> size_t i;
> diff --git a/lib/latch-unix.c b/lib/latch-unix.c
> index f4d10c39a..262f111e4 100644
> --- a/lib/latch-unix.c
> +++ b/lib/latch-unix.c
> @@ -71,7 +71,7 @@ latch_set(struct latch *latch)
> bool
> latch_is_set(const struct latch *latch)
> {
> - struct pollfd pfd;
> + struct pollfd pfd = {0, 0, 0};
> int retval;
>
> pfd.fd = latch->fds[0];
> diff --git a/lib/memory.c b/lib/memory.c
> index da97476c6..5ece04807 100644
> --- a/lib/memory.c
> +++ b/lib/memory.c
> @@ -116,13 +116,15 @@ compose_report(const struct simap *usage, struct ds *s)
> size_t n = simap_count(usage);
> size_t i;
>
> - for (i = 0; i < n; i++) {
> - const struct simap_node *node = nodes[i];
> + if (nodes) {
> + for (i = 0; i < n; i++) {
> + const struct simap_node *node = nodes[i];
>
> - ds_put_format(s, "%s:%u ", node->name, node->data);
> + ds_put_format(s, "%s:%u ", node->name, node->data);
> + }
> + free(nodes);
> }
> ds_chomp(s, ' ');
> - free(nodes);
> }
>
> /* Logs the contents of 'usage', as a collection of name-count pairs.
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index b89dfdd52..e2dd61880 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -221,16 +221,20 @@ netdev_tnl_calc_udp_csum(struct udp_header *udp, struct
> dp_packet *packet,
> {
> uint32_t csum;
>
> - if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> - csum = packet_csum_pseudoheader6(netdev_tnl_ipv6_hdr(
> - dp_packet_data(packet)));
> - } else {
> - csum = packet_csum_pseudoheader(netdev_tnl_ip_hdr(
> - dp_packet_data(packet)));
> - }
> + void *data_dp = dp_packet_data(packet);
> +
> + if (data_dp) {
> + if (netdev_tnl_is_header_ipv6(data_dp)) {
> + csum = packet_csum_pseudoheader6(netdev_tnl_ipv6_hdr(
> + data_dp));
> + } else {
> + csum = packet_csum_pseudoheader(netdev_tnl_ip_hdr(
> + data_dp));
> + }
>
> - csum = csum_continue(csum, udp, ip_tot_size);
> - udp->udp_csum = csum_finish(csum);
> + csum = csum_continue(csum, udp, ip_tot_size);
> + udp->udp_csum = csum_finish(csum);
> + }
>
> if (!udp->udp_csum) {
> udp->udp_csum = htons(0xffff);
> @@ -425,20 +429,24 @@ netdev_gre_pop_header(struct dp_packet *packet)
> struct flow_tnl *tnl = &md->tunnel;
> int hlen = sizeof(struct eth_header) + 4;
>
> - hlen += netdev_tnl_is_header_ipv6(dp_packet_data(packet)) ?
> - IPV6_HEADER_LEN : IP_HEADER_LEN;
> + const void *data_dp = dp_packet_data(packet);
>
> - pkt_metadata_init_tnl(md);
> - if (hlen > dp_packet_size(packet)) {
> - goto err;
> - }
> + if (data_dp) {
> + hlen += netdev_tnl_is_header_ipv6(data_dp) ?
> + IPV6_HEADER_LEN : IP_HEADER_LEN;
>
> - hlen = parse_gre_header(packet, tnl);
> - if (hlen < 0) {
> - goto err;
> - }
> + pkt_metadata_init_tnl(md);
> + if (hlen > dp_packet_size(packet)) {
> + goto err;
> + }
>
> - dp_packet_reset_packet(packet, hlen);
> + hlen = parse_gre_header(packet, tnl);
> + if (hlen < 0) {
> + goto err;
> + }
> +
> + dp_packet_reset_packet(packet, hlen);
> + }
>
> return packet;
> err:
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 5cf6fbec0..b8cf3db55 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -147,42 +147,45 @@ odp_set_ipv4(struct dp_packet *packet, const struct
> ovs_key_ipv4 *key,
> uint8_t new_tos;
> uint8_t new_ttl;
>
> - 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);
> + if (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);
>
> - if (ip_src_nh != new_ip_src) {
> - packet_set_ipv4_addr(packet, &nh->ip_src, new_ip_src);
> + if (ip_src_nh != new_ip_src) {
> + packet_set_ipv4_addr(packet, &nh->ip_src, new_ip_src);
> + }
> }
> - }
>
> - if (mask->ipv4_dst) {
> - ip_dst_nh = get_16aligned_be32(&nh->ip_dst);
> - new_ip_dst = key->ipv4_dst | (ip_dst_nh & ~mask->ipv4_dst);
> + if (mask->ipv4_dst) {
> + ip_dst_nh = get_16aligned_be32(&nh->ip_dst);
> + new_ip_dst = key->ipv4_dst | (ip_dst_nh & ~mask->ipv4_dst);
>
> - if (ip_dst_nh != new_ip_dst) {
> - packet_set_ipv4_addr(packet, &nh->ip_dst, new_ip_dst);
> + if (ip_dst_nh != new_ip_dst) {
> + packet_set_ipv4_addr(packet, &nh->ip_dst, new_ip_dst);
> + }
> }
> - }
>
> - if (mask->ipv4_tos) {
> - new_tos = key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos);
> + if (mask->ipv4_tos) {
> + new_tos = key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos);
>
> - if (nh->ip_tos != new_tos) {
> - nh->ip_csum = recalc_csum16(nh->ip_csum,
> - htons((uint16_t) nh->ip_tos),
> - htons((uint16_t) new_tos));
> - nh->ip_tos = new_tos;
> + if (nh->ip_tos != new_tos) {
> + nh->ip_csum = recalc_csum16(nh->ip_csum,
> + htons((uint16_t) nh->ip_tos),
> + htons((uint16_t) new_tos));
> + nh->ip_tos = new_tos;
> + }
> }
> - }
>
> - if (OVS_LIKELY(mask->ipv4_ttl)) {
> - new_ttl = key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl);
> + if (OVS_LIKELY(mask->ipv4_ttl)) {
> + new_ttl = key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl);
>
> - if (OVS_LIKELY(nh->ip_ttl != new_ttl)) {
> - nh->ip_csum = recalc_csum16(nh->ip_csum, htons(nh->ip_ttl << 8),
> - htons(new_ttl << 8));
> - nh->ip_ttl = new_ttl;
> + if (OVS_LIKELY(nh->ip_ttl != new_ttl)) {
> + nh->ip_csum = recalc_csum16(nh->ip_csum,
> + htons(nh->ip_ttl << 8),
> + htons(new_ttl << 8));
> + nh->ip_ttl = new_ttl;
> + }
> }
> }
> }
> @@ -276,23 +279,25 @@ set_arp(struct dp_packet *packet, const struct
> ovs_key_arp *key,
> {
> struct arp_eth_header *arp = dp_packet_l3(packet);
>
> - if (!mask) {
> - arp->ar_op = key->arp_op;
> - arp->ar_sha = key->arp_sha;
> - put_16aligned_be32(&arp->ar_spa, key->arp_sip);
> - arp->ar_tha = key->arp_tha;
> - put_16aligned_be32(&arp->ar_tpa, key->arp_tip);
> - } else {
> - ovs_be32 ar_spa = get_16aligned_be32(&arp->ar_spa);
> - ovs_be32 ar_tpa = get_16aligned_be32(&arp->ar_tpa);
> -
> - arp->ar_op = key->arp_op | (arp->ar_op & ~mask->arp_op);
> - ether_addr_copy_masked(&arp->ar_sha, key->arp_sha, mask->arp_sha);
> - put_16aligned_be32(&arp->ar_spa,
> - key->arp_sip | (ar_spa & ~mask->arp_sip));
> - ether_addr_copy_masked(&arp->ar_tha, key->arp_tha, mask->arp_tha);
> - put_16aligned_be32(&arp->ar_tpa,
> - key->arp_tip | (ar_tpa & ~mask->arp_tip));
> + if (arp) {
> + if (!mask) {
> + arp->ar_op = key->arp_op;
> + arp->ar_sha = key->arp_sha;
> + put_16aligned_be32(&arp->ar_spa, key->arp_sip);
> + arp->ar_tha = key->arp_tha;
> + put_16aligned_be32(&arp->ar_tpa, key->arp_tip);
> + } else {
> + ovs_be32 ar_spa = get_16aligned_be32(&arp->ar_spa);
> + ovs_be32 ar_tpa = get_16aligned_be32(&arp->ar_tpa);
> +
> + arp->ar_op = key->arp_op | (arp->ar_op & ~mask->arp_op);
> + ether_addr_copy_masked(&arp->ar_sha, key->arp_sha,
> mask->arp_sha);
> + put_16aligned_be32(&arp->ar_spa,
> + key->arp_sip | (ar_spa & ~mask->arp_sip));
> + ether_addr_copy_masked(&arp->ar_tha, key->arp_tha,
> mask->arp_tha);
> + put_16aligned_be32(&arp->ar_tpa,
> + key->arp_tip | (ar_tpa & ~mask->arp_tip));
> + }
> }
> }
>
> diff --git a/lib/pcap-file.c b/lib/pcap-file.c
> index 3ed7ea488..7cb6f5b2a 100644
> --- a/lib/pcap-file.c
> +++ b/lib/pcap-file.c
> @@ -291,7 +291,10 @@ ovs_pcap_write(struct pcap_file *p_file, struct
> dp_packet *buf)
> prh.incl_len = dp_packet_size(buf);
> prh.orig_len = dp_packet_size(buf);
> ignore(fwrite(&prh, sizeof prh, 1, p_file->file));
> - ignore(fwrite(dp_packet_data(buf), dp_packet_size(buf), 1,
> p_file->file));
> + const void *data_dp = dp_packet_data(buf);
> + if (data_dp) {
> + ignore(fwrite(data_dp, dp_packet_size(buf), 1, p_file->file));
> + }
> fflush(p_file->file);
> }
>
> diff --git a/lib/rtnetlink.c b/lib/rtnetlink.c
> index f67352603..67a0134ed 100644
> --- a/lib/rtnetlink.c
> +++ b/lib/rtnetlink.c
> @@ -131,10 +131,13 @@ rtnetlink_parse(struct ofpbuf *buf, struct
> rtnetlink_change *change)
> change->irrelevant = true;
> }
>
> + if (ifinfo) {
> + change->if_index = ifinfo->ifi_index;
> + change->ifi_flags = ifinfo->ifi_flags;
> + }
> +
> change->nlmsg_type = nlmsg->nlmsg_type;
> - change->if_index = ifinfo->ifi_index;
> change->ifname = nl_attr_get_string(attrs[IFLA_IFNAME]);
> - change->ifi_flags = ifinfo->ifi_flags;
> change->master_ifindex = (attrs[IFLA_MASTER]
> ? nl_attr_get_u32(attrs[IFLA_MASTER])
> : 0);
> @@ -178,7 +181,9 @@ rtnetlink_parse(struct ofpbuf *buf, struct
> rtnetlink_change *change)
> ifaddr = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *ifaddr);
>
> change->nlmsg_type = nlmsg->nlmsg_type;
> - change->if_index = ifaddr->ifa_index;
> + if (ifaddr) {
> + change->if_index = ifaddr->ifa_index;
> + }
> change->ifname = (attrs[IFA_LABEL]
> ? nl_attr_get_string(attrs[IFA_LABEL])
> : NULL);
> diff --git a/lib/sflow_agent.c b/lib/sflow_agent.c
> index c95f654a5..c19434d6c 100644
> --- a/lib/sflow_agent.c
> +++ b/lib/sflow_agent.c
> @@ -152,15 +152,24 @@ void sfl_agent_tick(SFLAgent *agent, time_t now)
>
> SFLReceiver *sfl_agent_addReceiver(SFLAgent *agent)
> {
> - SFLReceiver *rcv = (SFLReceiver *)sflAlloc(agent, sizeof(SFLReceiver));
> - sfl_receiver_init(rcv, agent);
> - /* add to end of list - to preserve the receiver index numbers for
> existing receivers */
> - {
> - SFLReceiver *r, *prev = NULL;
> - for(r = agent->receivers; r != NULL; prev = r, r = r->nxt);
> - if(prev) prev->nxt = rcv;
> - else agent->receivers = rcv;
> - rcv->nxt = NULL;
> + SFLReceiver *rcv = (SFLReceiver *) sflAlloc(agent, sizeof(SFLReceiver));
> + if (rcv) {
> + sfl_receiver_init(rcv, agent);
> + /* add to end of list - to preserve the receiver index numbers for
> + * existing receivers
> + */
> + {
> + SFLReceiver *r, *prev = NULL;
> + for (r = agent->receivers; r != NULL; prev = r, r = r->nxt) {
> + /* nothing to do */
> + }
> + if (prev) {
> + prev->nxt = rcv;
> + } else {
> + agent->receivers = rcv;
> + }
> + rcv->nxt = NULL;
> + }
> }
> return rcv;
> }
> @@ -202,23 +211,35 @@ SFLSampler *sfl_agent_addSampler(SFLAgent *agent,
> SFLDataSource_instance *pdsi)
> /* either we found the insert point, or reached the end of the list...*/
>
> {
> - SFLSampler *newsm = (SFLSampler *)sflAlloc(agent, sizeof(SFLSampler));
> - sfl_sampler_init(newsm, agent, pdsi);
> - if(prev) prev->nxt = newsm;
> - else agent->samplers = newsm;
> - newsm->nxt = sm;
> -
> - /* see if we should go in the ifIndex jumpTable */
> - if(SFL_DS_CLASS(newsm->dsi) == 0) {
> - SFLSampler *test = sfl_agent_getSamplerByIfIndex(agent,
> SFL_DS_INDEX(newsm->dsi));
> - if(test && (SFL_DS_INSTANCE(newsm->dsi) <
> SFL_DS_INSTANCE(test->dsi))) {
> - /* replace with this new one because it has a lower ds_instance
> number */
> - sfl_agent_jumpTableRemove(agent, test);
> - test = NULL;
> - }
> - if(test == NULL) sfl_agent_jumpTableAdd(agent, newsm);
> - }
> - return newsm;
> + SFLSampler *newsm = (SFLSampler *) sflAlloc(agent,
> sizeof(SFLSampler));
> + if (newsm) {
> + memset(newsm, 0, sizeof(*newsm));
> + sfl_sampler_init(newsm, agent, pdsi);
> + if (prev) {
> + prev->nxt = newsm;
> + } else {
> + agent->samplers = newsm;
> + }
> + newsm->nxt = sm;
> +
> + /* see if we should go in the ifIndex jumpTable */
> + if (SFL_DS_CLASS(newsm->dsi) == 0) {
> + SFLSampler *test = sfl_agent_getSamplerByIfIndex(agent,
> + SFL_DS_INDEX(newsm->dsi));
> + if (test && (SFL_DS_INSTANCE(newsm->dsi)
> + < SFL_DS_INSTANCE(test->dsi))) {
> + /* replace with this new one because it has a lower
> + * ds_instance number
> + */
> + sfl_agent_jumpTableRemove(agent, test);
> + test = NULL;
> + }
> + if (test == NULL) {
> + sfl_agent_jumpTableAdd(agent, newsm);
> + }
> + }
> + }
> + return newsm;
> }
> }
>
> @@ -241,12 +262,18 @@ SFLPoller *sfl_agent_addPoller(SFLAgent *agent,
> }
> /* either we found the insert point, or reached the end of the list... */
> {
> - SFLPoller *newpl = (SFLPoller *)sflAlloc(agent, sizeof(SFLPoller));
> - sfl_poller_init(newpl, agent, pdsi, magic, getCountersFn);
> - if(prev) prev->nxt = newpl;
> - else agent->pollers = newpl;
> - newpl->nxt = pl;
> - return newpl;
> + SFLPoller *newpl = (SFLPoller *) sflAlloc(agent, sizeof(SFLPoller));
> + if (newpl) {
> + memset(newpl, 0, sizeof(*newpl));
> + sfl_poller_init(newpl, agent, pdsi, magic, getCountersFn);
> + if (prev) {
> + prev->nxt = newpl;
> + } else {
> + agent->pollers = newpl;
> + }
> + newpl->nxt = pl;
> + }
> + return newpl;
> }
> }
>
> diff --git a/lib/shash.c b/lib/shash.c
> index a7b2c6458..adb388cf7 100644
> --- a/lib/shash.c
> +++ b/lib/shash.c
> @@ -194,7 +194,9 @@ shash_replace_nocopy(struct shash *sh, char *name, const
> void *data)
> void
> shash_delete(struct shash *sh, struct shash_node *node)
> {
> - free(shash_steal(sh, node));
> + if (node) {
> + free(shash_steal(sh, node));
> + }
> }
>
> /* Deletes 'node' from 'sh'. Neither the node's name nor its data is freed;
> diff --git a/lib/sset.c b/lib/sset.c
> index aa1790020..50f6676df 100644
> --- a/lib/sset.c
> +++ b/lib/sset.c
> @@ -261,8 +261,11 @@ char *
> sset_pop(struct sset *set)
> {
> const char *name = SSET_FIRST(set);
> - char *copy = xstrdup(name);
> - sset_delete(set, SSET_NODE_FROM_NAME(name));
> + char *copy = NULL;
> + if (name) {
> + copy = xstrdup(name);
> + sset_delete(set, SSET_NODE_FROM_NAME(name));
> + }
> return copy;
> }
>
> diff --git a/ovsdb/condition.c b/ovsdb/condition.c
> index d0016fa7f..f36531669 100644
> --- a/ovsdb/condition.c
> +++ b/ovsdb/condition.c
> @@ -47,7 +47,15 @@ ovsdb_clause_from_json(const struct ovsdb_table_schema *ts,
>
> /* Column and arg fields are not being used with boolean functions.
> * Use dummy values */
> - clause->column = ovsdb_table_schema_get_column(ts, "_uuid");
> + const struct ovsdb_column *uuid_column;
> + uuid_column = ovsdb_table_schema_get_column(ts, "_uuid");
> + if (uuid_column) {
> + clause->column = uuid_column;
> + } else {
> + return ovsdb_syntax_error(json,
> + "unknown column",
> + "No column _uuid in table schema.");
> + }
> clause->index = clause->column->index;
> ovsdb_datum_init_default(&clause->arg, &clause->column->type);
> return NULL;
> diff --git a/ovsdb/file.c b/ovsdb/file.c
> index 2d887e53e..bccb76098 100644
> --- a/ovsdb/file.c
> +++ b/ovsdb/file.c
> @@ -522,7 +522,12 @@ ovsdb_file_txn_add_row(struct ovsdb_file_txn *ftxn,
> }
>
> if (row) {
> - struct ovsdb_table *table = new ? new->table : old->table;
> + struct ovsdb_table *table = NULL;
> + if (new) {
> + table = new->table;
> + } else if (old) {
> + table = old->table;
> + }
> char uuid[UUID_LEN + 1];
>
> if (table != ftxn->table) {
> @@ -533,14 +538,23 @@ ovsdb_file_txn_add_row(struct ovsdb_file_txn *ftxn,
>
> /* Create JSON object for transaction on this table. */
> ftxn->table_json = json_object_create();
> - ftxn->table = table;
> - json_object_put(ftxn->json, table->schema->name,
> ftxn->table_json);
> + if (table) {
> + ftxn->table = table;
> + json_object_put(ftxn->json, table->schema->name,
> + ftxn->table_json);
> + }
> }
>
> /* Add row to transaction for this table. */
> - snprintf(uuid, sizeof uuid,
> - UUID_FMT, UUID_ARGS(ovsdb_row_get_uuid(new ? new : old)));
> - json_object_put(ftxn->table_json, uuid, row);
> + if (new) {
> + snprintf(uuid, sizeof uuid,
> + UUID_FMT, UUID_ARGS(ovsdb_row_get_uuid(new)));
> + json_object_put(ftxn->table_json, uuid, row);
> + } else if (old) {
> + snprintf(uuid, sizeof uuid,
> + UUID_FMT, UUID_ARGS(ovsdb_row_get_uuid(old)));
> + json_object_put(ftxn->table_json, uuid, row);
> + }
> }
> }
>
> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> index 17868f5b7..b1cbaa227 100644
> --- a/ovsdb/jsonrpc-server.c
> +++ b/ovsdb/jsonrpc-server.c
> @@ -1038,7 +1038,7 @@ ovsdb_jsonrpc_session_got_request(struct
> ovsdb_jsonrpc_session *s,
> request->id);
> } else if (!strcmp(request->method, "get_schema")) {
> struct ovsdb *db = ovsdb_jsonrpc_lookup_db(s, request, &reply);
> - if (!reply) {
> + if (db && !reply) {
> reply = jsonrpc_create_reply(ovsdb_schema_to_json(db->schema),
> request->id);
> }
> @@ -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,7 @@ 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 191befcae..1ea92cc0f 100644
> --- a/ovsdb/monitor.c
> +++ b/ovsdb/monitor.c
> @@ -478,7 +478,9 @@ ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
> struct ovsdb_monitor_column *c;
>
> mt = shash_find_data(&dbmon->tables, table->schema->name);
> -
> + if (!mt) {
> + return NULL;
> + }
> /* Check for column duplication. Return duplicated column name. */
> if (mt->columns_index_map[column->index] != -1) {
> return column->name;
> @@ -781,11 +783,15 @@ ovsdb_monitor_table_condition_update(
> const struct json *cond_json)
> {
> if (!condition) {
> - return NULL;
> + return ovsdb_syntax_error(cond_json, NULL,
> + "Parse error, condition empty.");
> }
>
> struct ovsdb_monitor_table_condition *mtc =
> shash_find_data(&condition->tables, table->schema->name);
> + if (!mtc) {
> + return NULL;
> + }
> struct ovsdb_error *error;
> struct ovsdb_condition cond = OVSDB_CONDITION_INITIALIZER(&cond);
>
> @@ -1279,7 +1285,9 @@ ovsdb_monitor_table_add_select(struct ovsdb_monitor
> *dbmon,
> struct ovsdb_monitor_table * mt;
>
> mt = shash_find_data(&dbmon->tables, table->schema->name);
> - mt->select |= select;
> + if (mt) {
> + mt->select |= select;
> + }
> }
>
> /*
> @@ -1329,8 +1337,23 @@ ovsdb_monitor_changes_update(const struct ovsdb_row
> *old,
> const struct ovsdb_monitor_table *mt,
> struct ovsdb_monitor_change_set_for_table *mcst)
> {
> - const struct uuid *uuid = ovsdb_row_get_uuid(new ? new : old);
> - struct ovsdb_monitor_row *change;
> + const struct uuid *uuid = NULL;
> +
> + if (!new && !old) {
> + return;
> + } else {
> + if (new) {
> + uuid = ovsdb_row_get_uuid(new);
> + } else if (old) {
> + uuid = ovsdb_row_get_uuid(old);
> + }
> + }
> +
> + if (!uuid) {
> + return;
> + }
> +
> + struct ovsdb_monitor_row *change = NULL;
>
> change = ovsdb_monitor_changes_row_find(mcst, uuid);
> if (!change) {
> @@ -1657,15 +1680,21 @@ ovsdb_monitor_hash(const struct ovsdb_monitor *dbmon,
> size_t basis)
> nodes = shash_sort(&dbmon->tables);
> n = shash_count(&dbmon->tables);
>
> + if (!nodes) {
> + return basis;
> + }
> +
> for (i = 0; i < n; i++) {
> struct ovsdb_monitor_table *mt = nodes[i]->data;
>
> - basis = hash_pointer(mt->table, basis);
> - basis = hash_3words(mt->select, mt->n_columns, basis);
> + if (mt) {
> + basis = hash_pointer(mt->table, basis);
> + basis = hash_3words(mt->select, mt->n_columns, basis);
>
> - for (j = 0; j < mt->n_columns; j++) {
> - basis = hash_pointer(mt->columns[j].column, basis);
> - basis = hash_2words(mt->columns[j].select, basis);
> + for (j = 0; j < mt->n_columns; j++) {
> + basis = hash_pointer(mt->columns[j].column, basis);
> + basis = hash_2words(mt->columns[j].select, basis);
> + }
> }
> }
> free(nodes);
> diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
> index f1b8d6491..ca2becd8d 100644
> --- a/ovsdb/ovsdb-client.c
> +++ b/ovsdb/ovsdb-client.c
> @@ -1223,17 +1223,25 @@ parse_monitor_columns(char *arg, const char *server,
> const char *database,
>
> n = shash_count(&table->columns);
> nodes = shash_sort(&table->columns);
> - for (i = 0; i < n; i++) {
> - const struct ovsdb_column *column = nodes[i]->data;
> - if (column->index != OVSDB_COL_UUID
> - && column->index != OVSDB_COL_VERSION) {
> - add_column(server, column, columns, columns_json);
> + if (nodes) {
> + for (i = 0; i < n; i++) {
> + const struct ovsdb_column *column = nodes[i]->data;
> + if (column->index != OVSDB_COL_UUID
> + && column->index != OVSDB_COL_VERSION) {
> + add_column(server, column, columns, columns_json);
> + }
> }
> + free(nodes);
> }
> - free(nodes);
>
> - add_column(server, ovsdb_table_schema_get_column(table, "_version"),
> - columns, columns_json);
> + const struct ovsdb_column *version_column =
> + ovsdb_table_schema_get_column(table, "_version");
> +
> + if (version_column) {
> + add_column(server, version_column, columns, columns_json);
> + } else {
> + VLOG_ERR("Table does not contain _version column.");
> + }
> }
>
> if (!initial || !insert || !delete || !modify) {
> @@ -1439,14 +1447,16 @@ do_monitor__(struct jsonrpc *rpc, const char
> *database,
> ovs_fatal(0, "ALL tables are not allowed with condition");
> }
>
> - for (i = 0; i < n; i++) {
> - struct ovsdb_table_schema *table = nodes[i]->data;
> + if (nodes) {
> + for (i = 0; i < n; i++) {
> + struct ovsdb_table_schema *table = nodes[i]->data;
>
> - add_monitored_table(argc, argv, server, database, NULL, table,
> - monitor_requests,
> - &mts, &n_mts, &allocated_mts);
> + add_monitored_table(argc, argv, server, database, NULL,
> table,
> + monitor_requests,
> + &mts, &n_mts, &allocated_mts);
> + }
> + free(nodes);
> }
> - free(nodes);
> }
>
> send_db_change_aware(rpc);
> @@ -1870,6 +1880,10 @@ do_dump(struct jsonrpc *rpc, const char *database,
> n_tables = shash_count(&schema->tables);
> }
>
> + if (!tables) {
> + goto end;
> + }
> +
> /* Construct transaction to retrieve entire database. */
> transaction = json_array_create_1(json_string_create(database));
> for (i = 0; i < n_tables; i++) {
> @@ -1929,8 +1943,9 @@ do_dump(struct jsonrpc *rpc, const char *database,
> }
>
> jsonrpc_msg_destroy(reply);
> - shash_destroy(&custom_columns);
> free(tables);
> +end:
> + shash_destroy(&custom_columns);
> ovsdb_schema_destroy(schema);
> }
>
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index 33ca4910d..02e024de4 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -1784,14 +1784,16 @@ ovsdb_server_list_databases(struct unixctl_conn
> *conn, int argc OVS_UNUSED,
> ds_init(&s);
>
> nodes = shash_sort(all_dbs);
> - for (i = 0; i < shash_count(all_dbs); i++) {
> - const struct shash_node *node = nodes[i];
> - struct db *db = node->data;
> - if (db->db) {
> - ds_put_format(&s, "%s\n", node->name);
> + if (nodes) {
> + for (i = 0; i < shash_count(all_dbs); i++) {
> + const struct shash_node *node = nodes[i];
> + struct db *db = node->data;
> + if (db->db) {
> + ds_put_format(&s, "%s\n", node->name);
> + }
> }
> + free(nodes);
> }
> - free(nodes);
>
> unixctl_command_reply(conn, ds_cstr(&s));
> ds_destroy(&s);
> @@ -2191,6 +2193,7 @@ 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/ovsdb-util.c b/ovsdb/ovsdb-util.c
> index 303191dc8..148230da8 100644
> --- a/ovsdb/ovsdb-util.c
> +++ b/ovsdb/ovsdb-util.c
> @@ -291,6 +291,15 @@ ovsdb_util_write_string_string_column(struct ovsdb_row
> *row,
> size_t i;
>
> column = ovsdb_table_schema_get_column(row->table->schema, column_name);
> + if (!column) {
> + VLOG_ERR("No %s column present in the %s table",
> + column_name, row->table->schema->name);
> + for (i = 0; i < n; i++) {
> + free(keys[i]);
> + free(values[i]);
> + }
> + return;
> + }
> datum = ovsdb_util_get_datum(row, column_name, OVSDB_TYPE_STRING,
> OVSDB_TYPE_STRING, UINT_MAX);
> if (!datum) {
> diff --git a/ovsdb/ovsdb.c b/ovsdb/ovsdb.c
> index afec96264..6dcfc5751 100644
> --- a/ovsdb/ovsdb.c
> +++ b/ovsdb/ovsdb.c
> @@ -195,7 +195,7 @@ root_set_size(const struct ovsdb_schema *schema)
> struct ovsdb_error *
> ovsdb_schema_from_json(const struct json *json, struct ovsdb_schema
> **schemap)
> {
> - struct ovsdb_schema *schema;
> + struct ovsdb_schema *schema = NULL;
> const struct json *name, *tables, *version_json, *cksum;
> struct ovsdb_error *error;
> struct shash_node *node;
> @@ -215,78 +215,86 @@ ovsdb_schema_from_json(const struct json *json, struct
> ovsdb_schema **schemap)
> return error;
> }
>
> - if (version_json) {
> - version = json_string(version_json);
> - if (!ovsdb_is_valid_version(version)) {
> - return ovsdb_syntax_error(json, NULL, "schema version \"%s\" not
> "
> - "in format x.y.z", version);
> + if (name && tables) {
> + if (version_json) {
> + version = json_string(version_json);
> + if (!ovsdb_is_valid_version(version)) {
> + return ovsdb_syntax_error(json, NULL,
> + "schema version \"%s\" not "
> + "in format x.y.z", version);
> + }
> + } else {
> + /* Backward compatibility with old databases. */
> + version = "";
> }
> - } else {
> - /* Backward compatibility with old databases. */
> - version = "";
> - }
>
> - schema = ovsdb_schema_create(json_string(name), version,
> - cksum ? json_string(cksum) : "");
> - SHASH_FOR_EACH (node, json_object(tables)) {
> - struct ovsdb_table_schema *table;
> + schema = ovsdb_schema_create(json_string(name), version,
> + cksum ? json_string(cksum) : "");
> + SHASH_FOR_EACH (node, json_object(tables)) {
> + struct ovsdb_table_schema *table;
> +
> + if (node->name[0] == '_') {
> + error = ovsdb_syntax_error(json, NULL, "names beginning with
> "
> + "\"_\" are reserved");
> + } else if (!ovsdb_parser_is_id(node->name)) {
> + error = ovsdb_syntax_error(json, NULL,
> + "name must be a valid id");
> + } else {
> + error = ovsdb_table_schema_from_json(node->data, node->name,
> + &table);
> + }
> + if (error) {
> + ovsdb_schema_destroy(schema);
> + return error;
> + }
>
> - if (node->name[0] == '_') {
> - error = ovsdb_syntax_error(json, NULL, "names beginning with "
> - "\"_\" are reserved");
> - } else if (!ovsdb_parser_is_id(node->name)) {
> - error = ovsdb_syntax_error(json, NULL, "name must be a valid
> id");
> - } else {
> - error = ovsdb_table_schema_from_json(node->data, node->name,
> - &table);
> - }
> - if (error) {
> - ovsdb_schema_destroy(schema);
> - return error;
> + shash_add(&schema->tables, table->name, table);
> }
> -
> - shash_add(&schema->tables, table->name, table);
> }
>
> - /* "isRoot" was not part of the original schema definition. Before it
> was
> - * added, there was no support for garbage collection. So, for backward
> - * compatibility, if the root set is empty then assume that every table
> is
> - * in the root set. */
> - if (root_set_size(schema) == 0) {
> - SHASH_FOR_EACH (node, &schema->tables) {
> - struct ovsdb_table_schema *table = node->data;
> -
> - table->is_root = true;
> + if (schema) {
> + /* "isRoot" was not part of the original schema definition. Before
> it
> + * was added, there was no support for garbage collection. So, for
> + * backward compatibility, if the root set is empty then assume that
> + * every table is in the root set. */
> + if (root_set_size(schema) == 0) {
> + SHASH_FOR_EACH (node, &schema->tables) {
> + struct ovsdb_table_schema *table = node->data;
> +
> + table->is_root = true;
> + }
> }
> - }
>
> - /* Validate that all refTables refer to the names of tables that exist.
> - *
> - * Also force certain columns to be persistent, as explained in
> - * ovsdb_schema_check_ref_table(). This requires 'is_root' to be known,
> so
> - * this must follow the loop updating 'is_root' above. */
> - SHASH_FOR_EACH (node, &schema->tables) {
> - struct ovsdb_table_schema *table = node->data;
> - struct shash_node *node2;
> + /* Validate that all refTables refer to the names of tables that
> exist.
> + *
> + * Also force certain columns to be persistent, as explained in
> + * ovsdb_schema_check_ref_table(). This requires 'is_root' to be
> + * known, so this must follow the loop updating 'is_root' above. */
> + SHASH_FOR_EACH (node, &schema->tables) {
> + struct ovsdb_table_schema *table = node->data;
> + struct shash_node *node2;
>
> - SHASH_FOR_EACH (node2, &table->columns) {
> - struct ovsdb_column *column = node2->data;
> + SHASH_FOR_EACH (node2, &table->columns) {
> + struct ovsdb_column *column = node2->data;
>
> - error = ovsdb_schema_check_ref_table(column, &schema->tables,
> - &column->type.key, "key");
> - if (!error) {
> error = ovsdb_schema_check_ref_table(column, &schema->tables,
> - &column->type.value,
> - "value");
> - }
> - if (error) {
> - ovsdb_schema_destroy(schema);
> - return error;
> + &column->type.key,
> "key");
> + if (!error) {
> + error = ovsdb_schema_check_ref_table(column,
> + &schema->tables,
> + &column->type.value,
> + "value");
> + }
> + if (error) {
> + ovsdb_schema_destroy(schema);
> + return error;
> + }
> }
> }
> +
> + *schemap = schema;
> }
>
> - *schemap = schema;
> return NULL;
> }
>
> diff --git a/ovsdb/query.c b/ovsdb/query.c
> index eebe56412..0129eb038 100644
> --- a/ovsdb/query.c
> +++ b/ovsdb/query.c
> @@ -91,7 +91,9 @@ ovsdb_query_distinct(struct ovsdb_table *table,
> struct ovsdb_row_hash hash;
>
> ovsdb_row_hash_init(&hash, columns);
> - ovsdb_query(table, condition, query_distinct_cb, &hash);
> + if (condition) {
> + ovsdb_query(table, condition, query_distinct_cb, &hash);
> + }
> HMAP_FOR_EACH (node, hmap_node, &hash.rows) {
> ovsdb_row_set_add_row(results, node->row);
> }
> diff --git a/ovsdb/replication.c b/ovsdb/replication.c
> index 477c69d70..f5e150c33 100644
> --- a/ovsdb/replication.c
> +++ b/ovsdb/replication.c
> @@ -575,15 +575,17 @@ create_monitor_request(struct ovsdb_schema *schema)
> size_t n = shash_count(&schema->tables);
> const struct shash_node **nodes = shash_sort(&schema->tables);
>
> - for (int j = 0; j < n; j++) {
> - struct ovsdb_table_schema *table = nodes[j]->data;
> + if (nodes) {
> + for (int j = 0; j < n; j++) {
> + struct ovsdb_table_schema *table = nodes[j]->data;
>
> - /* Monitor all tables not excluded. */
> - if (!excluded_tables_find(db_name, table->name)) {
> - add_monitored_table(table, monitor_request);
> + /* Monitor all tables not excluded. */
> + if (!excluded_tables_find(db_name, table->name)) {
> + add_monitored_table(table, monitor_request);
> + }
> }
> + free(nodes);
> }
> - free(nodes);
>
> /* Create a monitor request. */
> monitor = json_array_create_3(
> diff --git a/ovsdb/row.c b/ovsdb/row.c
> index d7bfbdd36..9f87c832a 100644
> --- a/ovsdb/row.c
> +++ b/ovsdb/row.c
> @@ -399,7 +399,9 @@ ovsdb_row_set_add_row(struct ovsdb_row_set *set, const
> struct ovsdb_row *row)
> set->rows = x2nrealloc(set->rows, &set->allocated_rows,
> sizeof *set->rows);
> }
> - set->rows[set->n_rows++] = row;
> + if (set->rows) {
> + set->rows[set->n_rows++] = row;
> + }
> }
>
> struct json *
> diff --git a/ovsdb/table.c b/ovsdb/table.c
> index 66071ce2f..990d49ea4 100644
> --- a/ovsdb/table.c
> +++ b/ovsdb/table.c
> @@ -158,7 +158,7 @@ ovsdb_table_schema_from_json(const struct json *json,
> const char *name,
> n_max_rows = UINT_MAX;
> }
>
> - if (shash_is_empty(json_object(columns))) {
> + if (!columns || shash_is_empty(json_object(columns))) {
> return ovsdb_syntax_error(json, NULL,
> "table must have at least one column");
> }
> diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
> index 03541af85..43b104f31 100644
> --- a/ovsdb/transaction.c
> +++ b/ovsdb/transaction.c
> @@ -576,9 +576,11 @@ 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(!ovsdb_row_find_weak_ref(dst_row, weak));
> - hmap_insert(&dst_row->dst_refs, &weak->dst_node,
> - ovsdb_weak_ref_hash(weak));
> + if (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));
> + }
> ovs_list_remove(&weak->src_node);
> ovs_list_init(&weak->src_node);
> }
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index 2f5ac1a26..991b426b9 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -334,8 +334,11 @@ parse_options(int argc, char *argv[], struct shash
> *local_options)
> exit(EXIT_SUCCESS);
>
> case 't':
> - if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
> - ctl_fatal("value %s on -t or --timeout is invalid", optarg);
> + if (optarg) {
> + if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
> + ctl_fatal("value %s on -t or --timeout is invalid",
> + optarg);
> + }
> }
> break;
>
> @@ -349,11 +352,15 @@ parse_options(int argc, char *argv[], struct shash
> *local_options)
> STREAM_SSL_OPTION_HANDLERS
>
> case OPT_PEER_CA_CERT:
> - stream_ssl_set_peer_ca_cert_file(optarg);
> + if (optarg) {
> + stream_ssl_set_peer_ca_cert_file(optarg);
> + }
> break;
>
> case OPT_BOOTSTRAP_CA_CERT:
> - stream_ssl_set_ca_cert_file(optarg, true);
> + if (optarg) {
> + stream_ssl_set_ca_cert_file(optarg, true);
> + }
> break;
>
> case '?':
> @@ -575,7 +582,7 @@ add_bridge_to_cache(struct vsctl_context *vsctl_ctx,
> struct ovsrec_bridge *br_cfg, const char *name,
> struct vsctl_bridge *parent, int vlan)
> {
> - struct vsctl_bridge *br = xmalloc(sizeof *br);
> + struct vsctl_bridge *br = xzalloc(sizeof *br);
> br->br_cfg = br_cfg;
> br->name = xstrdup(name);
> ovs_list_init(&br->ports);
> @@ -659,7 +666,7 @@ static struct vsctl_port *
> add_port_to_cache(struct vsctl_context *vsctl_ctx, struct vsctl_bridge
> *parent,
> struct ovsrec_port *port_cfg)
> {
> - struct vsctl_port *port;
> + struct vsctl_port *port = xzalloc(sizeof *port);
>
> if (port_cfg->tag
> && *port_cfg->tag >= 0 && *port_cfg->tag <= 4095) {
> @@ -671,11 +678,13 @@ add_port_to_cache(struct vsctl_context *vsctl_ctx,
> struct vsctl_bridge *parent,
> }
> }
>
> - port = xmalloc(sizeof *port);
> - ovs_list_push_back(&parent->ports, &port->ports_node);
> + if (parent) {
> + ovs_list_push_back(&parent->ports, &port->ports_node);
> + port->bridge = parent;
> + }
> +
> ovs_list_init(&port->ifaces);
> port->port_cfg = port_cfg;
> - port->bridge = parent;
> shash_add(&vsctl_ctx->ports, port_cfg->name, port);
>
> return port;
> @@ -818,55 +827,63 @@ vsctl_context_populate_cache(struct ctl_context *ctx)
> continue;
> }
> br = shash_find_data(&vsctl_ctx->bridges, br_cfg->name);
> - for (j = 0; j < br_cfg->n_ports; j++) {
> - struct ovsrec_port *port_cfg = br_cfg->ports[j];
> - struct vsctl_port *port;
> - size_t k;
> -
> - port = shash_find_data(&vsctl_ctx->ports, port_cfg->name);
> - if (port) {
> - if (port_cfg == port->port_cfg) {
> - VLOG_WARN("%s: port is in multiple bridges (%s and %s)",
> - port_cfg->name, br->name, port->bridge->name);
> - } else {
> - /* Log as an error because this violates the database's
> - * uniqueness constraints, so the database server
> shouldn't
> - * have allowed it. */
> - VLOG_ERR("%s: database contains duplicate port name",
> - port_cfg->name);
> - }
> - continue;
> - }
> -
> - if (port_is_fake_bridge(port_cfg)
> - && !sset_add(&bridges, port_cfg->name)) {
> - continue;
> - }
> -
> - port = add_port_to_cache(vsctl_ctx, br, port_cfg);
> - for (k = 0; k < port_cfg->n_interfaces; k++) {
> - struct ovsrec_interface *iface_cfg = port_cfg->interfaces[k];
> - struct vsctl_iface *iface;
> -
> - iface = shash_find_data(&vsctl_ctx->ifaces, iface_cfg->name);
> - if (iface) {
> - if (iface_cfg == iface->iface_cfg) {
> - VLOG_WARN("%s: interface is in multiple ports "
> - "(%s and %s)",
> - iface_cfg->name,
> - iface->port->port_cfg->name,
> - port->port_cfg->name);
> + if (br) {
> + for (j = 0; j < br_cfg->n_ports; j++) {
> + struct ovsrec_port *port_cfg = br_cfg->ports[j];
> + struct vsctl_port *port;
> + size_t k;
> +
> + port = shash_find_data(&vsctl_ctx->ports, port_cfg->name);
> + if (port) {
> + if (port_cfg == port->port_cfg) {
> + VLOG_WARN("%s: port is in multiple bridges "
> + "(%s and %s)", port_cfg->name, br->name,
> + port->bridge->name);
> } else {
> /* Log as an error because this violates the
> database's
> - * uniqueness constraints, so the database server
> - * shouldn't have allowed it. */
> - VLOG_ERR("%s: database contains duplicate interface "
> - "name", iface_cfg->name);
> + * uniqueness constraints, so the database server
> + * shouldn't have allowed it. */
> + VLOG_ERR("%s: database contains duplicate port name",
> + port_cfg->name);
> }
> continue;
> }
>
> - add_iface_to_cache(vsctl_ctx, port, iface_cfg);
> + if (port_is_fake_bridge(port_cfg)
> + && !sset_add(&bridges, port_cfg->name)) {
> + continue;
> + }
> +
> + port = add_port_to_cache(vsctl_ctx, br, port_cfg);
> + if (port) {
> + for (k = 0; k < port_cfg->n_interfaces; k++) {
> + struct ovsrec_interface *iface_cfg =
> + port_cfg->interfaces[k];
> + struct vsctl_iface *iface;
> +
> + iface = shash_find_data(&vsctl_ctx->ifaces,
> + iface_cfg->name);
> + if (iface) {
> + if (iface_cfg == iface->iface_cfg) {
> + VLOG_WARN("%s: interface is in multiple
> ports "
> + "(%s and %s)",
> + iface_cfg->name,
> + iface->port->port_cfg->name,
> + port->port_cfg->name);
> + } else {
> + /* Log as an error because this violates the
> + * database's uniqueness constraints, so the
> + * database server shouldn't have allowed it.
> + */
> + VLOG_ERR("%s: database contains duplicate "
> + "interface name", iface_cfg->name);
> + }
> + continue;
> + }
> +
> + add_iface_to_cache(vsctl_ctx, port, iface_cfg);
> + }
> + }
> }
> }
> }
> @@ -889,14 +906,23 @@ check_conflicts(struct vsctl_context *vsctl_ctx, const
> char *name,
>
> port = shash_find_data(&vsctl_ctx->ports, name);
> if (port) {
> - ctl_fatal("%s because a port named %s already exists on "
> - "bridge %s", msg, name, port->bridge->name);
> + if (port->bridge) {
> + ctl_fatal("%s because a port named %s already exists on "
> + "bridge %s", msg, name, port->bridge->name);
> + } else {
> + ctl_fatal("%s because a port named %s already exists", msg,
> name);
> + }
> }
>
> iface = shash_find_data(&vsctl_ctx->ifaces, name);
> if (iface) {
> - ctl_fatal("%s because an interface named %s already exists "
> - "on bridge %s", msg, name, iface->port->bridge->name);
> + if (iface->port->bridge) {
> + ctl_fatal("%s because an interface named %s already exists "
> + "on bridge %s", msg, name, iface->port->bridge->name);
> + } else {
> + ctl_fatal("%s because an interface named %s already exists", msg,
> + name);
> + }
> }
>
> free(msg);
> @@ -936,7 +962,7 @@ find_port(struct vsctl_context *vsctl_ctx, const char
> *name, bool must_exist)
> ovs_assert(vsctl_ctx->cache_valid);
>
> port = shash_find_data(&vsctl_ctx->ports, name);
> - if (port && !strcmp(name, port->bridge->name)) {
> + if (port && port->bridge && !strcmp(name, port->bridge->name)) {
> port = NULL;
> }
> if (must_exist && !port) {
> @@ -954,7 +980,8 @@ find_iface(struct vsctl_context *vsctl_ctx, const char
> *name, bool must_exist)
> ovs_assert(vsctl_ctx->cache_valid);
>
> iface = shash_find_data(&vsctl_ctx->ifaces, name);
> - if (iface && !strcmp(name, iface->port->bridge->name)) {
> + if (iface && iface->port->bridge &&
> + !strcmp(name, iface->port->bridge->name)) {
> iface = NULL;
> }
> if (must_exist && !iface) {
> @@ -1679,14 +1706,16 @@ get_external_id(struct smap *smap, const char
> *prefix, const char *key,
> size_t prefix_len = strlen(prefix);
> size_t i;
>
> - for (i = 0; i < smap_count(smap); i++) {
> - const struct smap_node *node = sorted[i];
> - if (!strncmp(node->key, prefix, prefix_len)) {
> - ds_put_format(output, "%s=%s\n", node->key + prefix_len,
> - node->value);
> + if (sorted) {
> + for (i = 0; i < smap_count(smap); i++) {
> + const struct smap_node *node = sorted[i];
> + if (!strncmp(node->key, prefix, prefix_len)) {
> + ds_put_format(output, "%s=%s\n", node->key + prefix_len,
> + node->value);
> + }
> }
> + free(sorted);
> }
> - free(sorted);
> }
> }
>
> diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
> index e5d99714d..356840089 100644
> --- a/vtep/vtep-ctl.c
> +++ b/vtep/vtep-ctl.c
> @@ -250,8 +250,11 @@ parse_options(int argc, char *argv[], struct shash
> *local_options)
> exit(EXIT_SUCCESS);
>
> case 't':
> - if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
> - ctl_fatal("value %s on -t or --timeout is invalid", optarg);
> + if (optarg) {
> + if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
> + ctl_fatal("value %s on -t or --timeout is invalid",
> + optarg);
> + }
> }
> break;
>
> @@ -1065,42 +1068,46 @@ vtep_ctl_context_populate_cache(struct ctl_context
> *ctx)
> continue;
> }
> ps = shash_find_data(&vtepctl_ctx->pswitches, ps_cfg->name);
> - for (j = 0; j < ps_cfg->n_ports; j++) {
> - struct vteprec_physical_port *port_cfg = ps_cfg->ports[j];
> - struct vtep_ctl_port *port;
> - size_t k;
> -
> - port = shash_find_data(&vtepctl_ctx->ports, port_cfg->name);
> - if (port) {
> - if (port_cfg == port->port_cfg) {
> - VLOG_WARN("%s: port is in multiple physical switches "
> - "(%s and %s)",
> - port_cfg->name, ps->name, port->ps->name);
> - } else {
> - /* Log as an error because this violates the database's
> - * uniqueness constraints, so the database server
> shouldn't
> - * have allowed it. */
> - VLOG_ERR("%s: database contains duplicate port name",
> - port_cfg->name);
> + if (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;
> + size_t k;
> +
> + port = shash_find_data(&vtepctl_ctx->ports, port_cfg->name);
> + if (port) {
> + if (port_cfg == port->port_cfg) {
> + VLOG_WARN("%s: port is in multiple physical switches
> "
> + "(%s and %s)",
> + port_cfg->name, ps->name, port->ps->name);
> + } else {
> + /* Log as an error because this violates the
> database's
> + * uniqueness constraints, so the database server
> + * shouldn't have allowed it. */
> + VLOG_ERR("%s: database contains duplicate port name",
> + port_cfg->name);
> + }
> + continue;
> }
> - continue;
> - }
>
> - port = add_port_to_cache(vtepctl_ctx, ps, port_cfg);
> + port = add_port_to_cache(vtepctl_ctx, ps, port_cfg);
> + if (port) {
> + for (k = 0; k < port_cfg->n_vlan_bindings; k++) {
> + struct vtep_ctl_lswitch *ls;
> + char *vlan;
>
> - for (k = 0; k < port_cfg->n_vlan_bindings; k++) {
> - struct vtep_ctl_lswitch *ls;
> - char *vlan;
> + vlan = xasprintf("%"PRId64,
> + port_cfg->key_vlan_bindings[k]);
> + if (shash_find(&port->bindings, vlan)) {
> + ctl_fatal("multiple bindings for vlan %s", vlan);
> + }
>
> - vlan = xasprintf("%"PRId64, port_cfg->key_vlan_bindings[k]);
> - if (shash_find(&port->bindings, vlan)) {
> - ctl_fatal("multiple bindings for vlan %s", vlan);
> - }
> -
> - ls_cfg = port_cfg->value_vlan_bindings[k];
> - ls = find_lswitch(vtepctl_ctx, ls_cfg->name, true);
> + ls_cfg = port_cfg->value_vlan_bindings[k];
> + ls = find_lswitch(vtepctl_ctx, ls_cfg->name, true);
>
> - shash_add_nocopy(&port->bindings, vlan, ls);
> + shash_add_nocopy(&port->bindings, vlan, ls);
> + }
> + }
> }
> }
> }
> @@ -1892,8 +1899,10 @@ del_mcast_entry(struct ctl_context *ctx,
> vteprec_mcast_macs_remote_delete(mcast_mac->remote_cfg);
> }
>
> - free(node->data);
> - shash_delete(mcast_shash, node);
> + if (node) {
> + free(node->data);
> + shash_delete(mcast_shash, node);
> + }
> } else {
> if (local) {
> vteprec_mcast_macs_local_set_locator_set(mcast_mac->local_cfg,
> --
> 2.39.2
>
> _______________________________________________
> 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
