Thanks, Ales.

Acked-by: Mark Michelson <mmich...@redhat.com>

On 1/4/24 07:54, Ales Musil wrote:
Fix a couple of small memory leaks that were reported by coverity
static analysis on ovn rpm package.

Signed-off-by: Ales Musil <amu...@redhat.com>
---
v2: Rebase on top of current main.
     Address comment from Dumitru:
     - Update the pinctrl_handle_empty_lb_backends_opts() to have
       only single point of cleanup.
---
  controller-vtep/ovn-controller-vtep.c |  2 +
  controller/lflow.c                    |  1 +
  controller/ovn-controller.c           |  1 +
  controller/pinctrl.c                  | 60 +++++++++++++++------------
  utilities/ovn-nbctl.c                 |  1 +
  utilities/ovn-trace.c                 |  1 +
  6 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/controller-vtep/ovn-controller-vtep.c 
b/controller-vtep/ovn-controller-vtep.c
index 23b368179..4472e5285 100644
--- a/controller-vtep/ovn-controller-vtep.c
+++ b/controller-vtep/ovn-controller-vtep.c
@@ -306,10 +306,12 @@ parse_options(int argc, char *argv[])
switch (c) {
          case 'd':
+            free(ovnsb_remote);
              ovnsb_remote = xstrdup(optarg);
              break;
case 'D':
+            free(vtep_remote);
              vtep_remote = xstrdup(optarg);
              break;
diff --git a/controller/lflow.c b/controller/lflow.c
index 85a4943dc..b0cf4253c 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -1017,6 +1017,7 @@ convert_match_to_expr(const struct sbrec_logical_flow 
*lflow,
          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
          VLOG_WARN_RL(&rl, "error parsing match \"%s\": %s",
                      lflow->match, error);
+        expr_destroy(e);
          free(error);
          return NULL;
      }
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 760b7788b..7cd146f48 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -6186,6 +6186,7 @@ parse_options(int argc, char *argv[])
              break;
case 'n':
+            free(cli_system_id);
              cli_system_id = xstrdup(optarg);
              break;
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 5a35d56f6..64a15ed02 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6404,26 +6404,31 @@ pinctrl_handle_empty_lb_backends_opts(struct ofpbuf 
*userdata)
      char *protocol = NULL;
      char *load_balancer = NULL;
+ struct empty_lb_backends_event *event = NULL;
+
      while (userdata->size) {
          userdata_opt = ofpbuf_try_pull(userdata, sizeof opt_hdr);
          if (!userdata_opt) {
-            return false;
+            goto cleanup;
          }
          memcpy(&opt_hdr, userdata_opt, sizeof opt_hdr);
size_t size = ntohs(opt_hdr.size);
          char *userdata_opt_data = ofpbuf_try_pull(userdata, size);
          if (!userdata_opt_data) {
-            return false;
+            goto cleanup;
          }
          switch (ntohs(opt_hdr.opt_code)) {
          case EMPTY_LB_VIP:
+            free(vip);
              vip = xmemdup0(userdata_opt_data, size);
              break;
          case EMPTY_LB_PROTOCOL:
+            free(protocol);
              protocol = xmemdup0(userdata_opt_data, size);
              break;
          case EMPTY_LB_LOAD_BALANCER:
+            free(load_balancer);
              load_balancer = xmemdup0(userdata_opt_data, size);
              break;
          default:
@@ -6434,36 +6439,39 @@ pinctrl_handle_empty_lb_backends_opts(struct ofpbuf 
*userdata)
      if (!vip || !protocol || !load_balancer) {
          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
          VLOG_WARN_RL(&rl, "missing lb parameters in userdata");
-        free(vip);
-        free(protocol);
-        free(load_balancer);
-        return false;
+        goto cleanup;
      }
- struct empty_lb_backends_event *event;
-
      event = pinctrl_find_empty_lb_backends_event(vip, protocol,
                                                   load_balancer, hash);
-    if (!event) {
-        if (hmap_count(&event_table[OVN_EVENT_EMPTY_LB_BACKENDS]) >= 1000) {
-            COVERAGE_INC(pinctrl_drop_controller_event);
-            return false;
-        }
+    if (event) {
+        goto cleanup;
+    }
- event = xzalloc(sizeof *event);
-        hmap_insert(&event_table[OVN_EVENT_EMPTY_LB_BACKENDS],
-                    &event->hmap_node, hash);
-        event->vip = vip;
-        event->protocol = protocol;
-        event->load_balancer = load_balancer;
-        event->timestamp = time_msec();
-        notify_pinctrl_main();
-    } else {
-        free(vip);
-        free(protocol);
-        free(load_balancer);
+    if (hmap_count(&event_table[OVN_EVENT_EMPTY_LB_BACKENDS]) >= 1000) {
+        COVERAGE_INC(pinctrl_drop_controller_event);
+        goto cleanup;
      }
-    return true;
+
+    event = xzalloc(sizeof *event);
+    hmap_insert(&event_table[OVN_EVENT_EMPTY_LB_BACKENDS],
+                &event->hmap_node, hash);
+    event->vip = vip;
+    event->protocol = protocol;
+    event->load_balancer = load_balancer;
+    event->timestamp = time_msec();
+    notify_pinctrl_main();
+
+    vip = NULL;
+    protocol = NULL;
+    load_balancer = NULL;
+
+cleanup:
+    free(vip);
+    free(protocol);
+    free(load_balancer);
+
+    return event != NULL;
  }
static void
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 9522078c1..526369b68 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4694,6 +4694,7 @@ static void
              nexthop = normalize_prefix_str(ctx->argv[3]);
              if (!nexthop) {
                  ctl_error(ctx, "bad nexthop argument: %s", ctx->argv[3]);
+                free(prefix);
                  return;
              }
          }
diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
index 0b86eae7b..13ae464ad 100644
--- a/utilities/ovn-trace.c
+++ b/utilities/ovn-trace.c
@@ -983,6 +983,7 @@ parse_lflow_for_datapath(const struct sbrec_logical_flow 
*sblf,
          if (error) {
              VLOG_WARN("%s: parsing expression failed (%s)",
                        sblf->match, error);
+            expr_destroy(match);
              free(error);
              return;
          }

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

Reply via email to