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>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
amu...@redhat.com
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
