| Message ID | 20240104142845.205691-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] treewide: Cleanup free() calls. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Thu, Jan 4, 2024 at 3:29 PM Dumitru Ceara <dceara@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 <dceara@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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
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 <dceara@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 <dceara@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 >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > Looks good to me, thanks. > > Acked-by: Ales Musil <amusil@redhat.com> >
On 1/4/24 20:56, Mark Michelson wrote: > Thanks Ales and Dumitru. > Thanks Mark! > 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. > I don't think it's really needed right now. Maybe just if we realize it causes conflicts with other backports. Otherwise main is fine. Regards, Dumitru > Mark Michelson > > On 1/4/24 12:17, Ales Musil wrote: >> On Thu, Jan 4, 2024 at 3:29 PM Dumitru Ceara <dceara@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 <dceara@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 >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >>> >> Looks good to me, thanks. >> >> Acked-by: Ales Musil <amusil@redhat.com> >> >
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
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 <dceara@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(-)