| Message ID | 20201120001724.2424494-14-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Pack of fixes for memory leaks. | expand |
On 11/20/20 1:17 AM, Ilya Maximets wrote: > Fixes: 742474bad730 ("ovn-nbctl: Enhance lr-policy-add to set the options.") > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > utilities/ovn-nbctl.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index 526dbf86c..448c86f42 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -3689,6 +3689,7 @@ nbctl_lr_policy_add(struct ctl_context *ctx) > } else { > ctl_error(ctx, "No value specified for the option : %s", key); > free(key); > + free(next_hop); Looks like we also need to: smap_destroy(&options); Thanks, Dumitru
On 11/20/20 6:12 PM, Dumitru Ceara wrote: > On 11/20/20 1:17 AM, Ilya Maximets wrote: >> Fixes: 742474bad730 ("ovn-nbctl: Enhance lr-policy-add to set the options.") >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >> --- >> utilities/ovn-nbctl.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c >> index 526dbf86c..448c86f42 100644 >> --- a/utilities/ovn-nbctl.c >> +++ b/utilities/ovn-nbctl.c >> @@ -3689,6 +3689,7 @@ nbctl_lr_policy_add(struct ctl_context *ctx) >> } else { >> ctl_error(ctx, "No value specified for the option : %s", key); >> free(key); >> + free(next_hop); > > Looks like we also need to: > > smap_destroy(&options); Good catch. We also need to free 'value', I guess, because an empty string still consumes some memory (at least one byte). This might be fixed in a separate patch, though. Dumitru, Numan, what do you think? Best regards, Ilya Maximets.
On 11/20/20 6:52 PM, Ilya Maximets wrote: > On 11/20/20 6:12 PM, Dumitru Ceara wrote: >> On 11/20/20 1:17 AM, Ilya Maximets wrote: >>> Fixes: 742474bad730 ("ovn-nbctl: Enhance lr-policy-add to set the options.") >>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >>> --- >>> utilities/ovn-nbctl.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c >>> index 526dbf86c..448c86f42 100644 >>> --- a/utilities/ovn-nbctl.c >>> +++ b/utilities/ovn-nbctl.c >>> @@ -3689,6 +3689,7 @@ nbctl_lr_policy_add(struct ctl_context *ctx) >>> } else { >>> ctl_error(ctx, "No value specified for the option : %s", key); >>> free(key); >>> + free(next_hop); >> >> Looks like we also need to: >> >> smap_destroy(&options); > > Good catch. We also need to free 'value', I guess, because an empty > string still consumes some memory (at least one byte). > > This might be fixed in a separate patch, though. > > Dumitru, Numan, what do you think? Sounds good to me, thanks!
On Fri, Nov 20, 2020 at 11:35 PM Dumitru Ceara <dceara@redhat.com> wrote: > > On 11/20/20 6:52 PM, Ilya Maximets wrote: > > On 11/20/20 6:12 PM, Dumitru Ceara wrote: > >> On 11/20/20 1:17 AM, Ilya Maximets wrote: > >>> Fixes: 742474bad730 ("ovn-nbctl: Enhance lr-policy-add to set the options.") > >>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > >>> --- > >>> utilities/ovn-nbctl.c | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > >>> index 526dbf86c..448c86f42 100644 > >>> --- a/utilities/ovn-nbctl.c > >>> +++ b/utilities/ovn-nbctl.c > >>> @@ -3689,6 +3689,7 @@ nbctl_lr_policy_add(struct ctl_context *ctx) > >>> } else { > >>> ctl_error(ctx, "No value specified for the option : %s", key); > >>> free(key); > >>> + free(next_hop); > >> > >> Looks like we also need to: > >> > >> smap_destroy(&options); > > > > Good catch. We also need to free 'value', I guess, because an empty > > string still consumes some memory (at least one byte). > > > > This might be fixed in a separate patch, though. > > > > Dumitru, Numan, what do you think? > > Sounds good to me, thanks! Sounds good to me too. Numan > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 526dbf86c..448c86f42 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -3689,6 +3689,7 @@ nbctl_lr_policy_add(struct ctl_context *ctx) } else { ctl_error(ctx, "No value specified for the option : %s", key); free(key); + free(next_hop); return; } free(key);
Fixes: 742474bad730 ("ovn-nbctl: Enhance lr-policy-add to set the options.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- utilities/ovn-nbctl.c | 1 + 1 file changed, 1 insertion(+)