Thanks Ales and Dumitru.

I applied this patch to main only, since it's not really fixing any problems. If you think this should be backported, let me know and I can apply it to older branches.

Mark Michelson

On 1/4/24 12:17, Ales Musil wrote:
On Thu, Jan 4, 2024 at 3:29 PM Dumitru Ceara <dce...@redhat.com> wrote:

It's perfectly fine to call free(NULL).  Take advantage of that to
simplify the code.  Use the safer CONST_CAST way of "unconsting"
pointers we pass to free while we're at it.

Signed-off-by: Dumitru Ceara <dce...@redhat.com>
---
  controller/ovn-controller.c | 12 +++---------
  controller/pinctrl.c        |  5 +----
  controller/vif-plug.c       | 12 +++---------
  lib/expr.c                  | 22 ++++++++--------------
  lib/ovn-parallel-hmap.c     |  6 ++----
  northd/ipam.c               |  2 +-
  6 files changed, 18 insertions(+), 41 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 760b7788be..8709c1ae2a 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -688,9 +688,7 @@ add_pending_ct_zone_entry(struct shash
*pending_ct_zones,
       */
      struct ct_zone_pending_entry *existing =
          shash_replace(pending_ct_zones, name, pending);
-    if (existing) {
-        free(existing);
-    }
+    free(existing);
  }

  static bool
@@ -6099,12 +6097,8 @@ loop_done:

      ovs_feature_support_destroy();
      free(ovs_remote);
-    if (file_system_id) {
-        free(file_system_id);
-    }
-    if (cli_system_id) {
-        free(cli_system_id);
-    }
+    free(file_system_id);
+    free(cli_system_id);
      ovn_exit_args_finish(&exit_args);
      unixctl_server_destroy(unixctl);
      service_stop();
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 5a35d56f6b..12055a6756 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -1282,10 +1282,7 @@ fill_ipv6_prefix_state(struct ovsdb_idl_txn
*ovnsb_idl_txn,
          struct ipv6_prefixd_state *pfd;

          if (!smap_get_bool(&pb->options, "ipv6_prefix", false)) {
-            pfd = shash_find_and_delete(&ipv6_prefixd, pb->logical_port);
-            if (pfd) {
-                free(pfd);
-            }
+            free(shash_find_and_delete(&ipv6_prefixd, pb->logical_port));
              continue;
          }

diff --git a/controller/vif-plug.c b/controller/vif-plug.c
index 38348bf544..d4c7552eab 100644
--- a/controller/vif-plug.c
+++ b/controller/vif-plug.c
@@ -142,15 +142,9 @@ destroy_port_ctx(struct vif_plug_port_ctx *ctx)
  {
      smap_destroy(&ctx->vif_plug_port_ctx_in.lport_options);
      smap_destroy(&ctx->vif_plug_port_ctx_in.iface_options);
-    if (ctx->vif_plug_port_ctx_in.lport_name) {
-        free((char *)ctx->vif_plug_port_ctx_in.lport_name);
-    }
-    if (ctx->vif_plug_port_ctx_in.iface_name) {
-        free((char *)ctx->vif_plug_port_ctx_in.iface_name);
-    }
-    if (ctx->vif_plug_port_ctx_in.iface_type) {
-        free((char *)ctx->vif_plug_port_ctx_in.iface_type);
-    }
+    free(CONST_CAST(char *, ctx->vif_plug_port_ctx_in.lport_name));
+    free(CONST_CAST(char *, ctx->vif_plug_port_ctx_in.iface_name));
+    free(CONST_CAST(char *, ctx->vif_plug_port_ctx_in.iface_type));
      /* Note that data associated with ctx->vif_plug_port_ctx_out must be
       * destroyed by the plug provider implementation with a call to
       * vif_plug_port_ctx_destroy prior to calling this function */
diff --git a/lib/expr.c b/lib/expr.c
index f5d2032334..0cb44e1b6f 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -179,17 +179,13 @@ expr_combine(enum expr_type type, struct expr *a,
struct expr *b)
          } else {
              ovs_list_push_back(&a->andor, &b->node);
          }
-        if (a->as_name) {
-            free(a->as_name);
-            a->as_name = NULL;
-        }
+        free(a->as_name);
+        a->as_name = NULL;
          return a;
      } else if (b->type == type) {
          ovs_list_push_front(&b->andor, &a->node);
-        if (b->as_name) {
-            free(b->as_name);
-            b->as_name = NULL;
-        }
+        free(b->as_name);
+        b->as_name = NULL;
          return b;
      } else {
          struct expr *e = expr_create_andor(type);
@@ -879,12 +875,10 @@ parse_constant(struct expr_context *ctx, struct
expr_constant_set *cs,
                                  sizeof *cs->values);
      }

-    if (cs->as_name) {
-        /* Combining other values to the constant set that is tracking an
-         * address set, so untrack it. */
-        free(cs->as_name);
-        cs->as_name = NULL;
-    }
+    /* Combining other values to the constant set that is tracking an
+     * address set, so untrack it. */
+    free(cs->as_name);
+    cs->as_name = NULL;

      if (ctx->lexer->token.type == LEX_T_TEMPLATE) {
          lexer_error(ctx->lexer, "Unexpanded template.");
diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c
index a0ba681a1e..1b167e2145 100644
--- a/lib/ovn-parallel-hmap.c
+++ b/lib/ovn-parallel-hmap.c
@@ -427,10 +427,8 @@ ovn_update_hashrow_locks(struct hmap *lflows, struct
hashrow_locks *hrl)
  {
      int i;
      if (hrl->mask != lflows->mask) {
-        if (hrl->row_locks) {
-            free(hrl->row_locks);
-        }
-        hrl->row_locks = xcalloc(sizeof(struct ovs_mutex), lflows->mask +
1);
+        hrl->row_locks = xrealloc(hrl->row_locks,
+                                  sizeof *hrl->row_locks * (lflows->mask
+ 1));
          hrl->mask = lflows->mask;
          for (i = 0; i <= lflows->mask; i++) {
              ovs_mutex_init(&hrl->row_locks[i]);
diff --git a/northd/ipam.c b/northd/ipam.c
index 68a757098b..4448a76074 100644
--- a/northd/ipam.c
+++ b/northd/ipam.c
@@ -44,7 +44,7 @@ void
  destroy_ipam_info(struct ipam_info *info)
  {
      bitmap_free(info->allocated_ipv4s);
-    free((char *) info->id);
+    free(CONST_CAST(char *, info->id));
  }

  bool
--
2.39.3

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Looks good to me, thanks.

Acked-by: Ales Musil <amu...@redhat.com>


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to