[ovs-dev,1/1] ovn-northd: fixed memory leak in ovn_port_update_sbrec()
diff mbox series

Message ID 1564748041-12962-1-git-send-email-damjan.skvarc@gmail.com
State New
Headers show
Series
  • [ovs-dev,1/1] ovn-northd: fixed memory leak in ovn_port_update_sbrec()
Related show

Commit Message

Damijan Skvarc Aug. 2, 2019, 12:14 p.m. UTC
Memory leak happens because of redundand memory allocation for array
of single pointer. Issue was solved by removing this redundand allocation
and using address of pointer to created chassis sb_ha_entity instead.

Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com>
---
 northd/ovn-northd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

0-day Robot Aug. 2, 2019, 12:58 p.m. UTC | #1
Bleep bloop.  Greetings Damijan Skvarc, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
fatal: sha1 information is lacking or useless (northd/ovn-northd.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 ovn-northd: fixed memory leak in ovn_port_update_sbrec()
The copy of the patch that failed is found in:
   /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Numan Siddique Aug. 2, 2019, 4:13 p.m. UTC | #2
On Fri, Aug 2, 2019 at 5:45 PM Damijan Skvarc <damjan.skvarc@gmail.com>
wrote:

> Memory leak happens because of redundand memory allocation for array
> of single pointer. Issue was solved by removing this redundand allocation
> and using address of pointer to created chassis sb_ha_entity instead.
>
> Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com>
>

Acked-by: Numan Siddique <nusiddiq@redhat.com>



> ---
>  northd/ovn-northd.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index cd776fa..d99ae67 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -2506,12 +2506,10 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>                      }
>
>                      if (sb_ha_ch_grp->n_ha_chassis != 1) {
> -                        struct sbrec_ha_chassis **sb_ha_ch =
> -                            xcalloc(1, sizeof *sb_ha_ch);
> -                        sb_ha_ch[0] = create_sb_ha_chassis(ctx, chassis,
> +                        struct sbrec_ha_chassis *sb_ha_ch =
> create_sb_ha_chassis(ctx, chassis,
>                                                             chassis->name,
> 0);
>
>  sbrec_ha_chassis_group_set_ha_chassis(sb_ha_ch_grp,
> -                                                              sb_ha_ch,
> 1);
> +                                                              &sb_ha_ch,
> 1);
>                      }
>                      sbrec_port_binding_set_ha_chassis_group(op->sb,
>                                                              sb_ha_ch_grp);
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Numan Siddique Aug. 2, 2019, 4:15 p.m. UTC | #3
On Fri, Aug 2, 2019 at 6:31 PM 0-day Robot <robot@bytheb.org> wrote:

> Bleep bloop.  Greetings Damijan Skvarc, I am a robot and I have tried out
> your patch.
> Thanks for your contribution.
>
> I encountered some error that I wasn't expecting.  See the details below.
>
>
> git-am:
> fatal: sha1 information is lacking or useless (northd/ovn-northd.c).
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0001 ovn-northd: fixed memory leak in
> ovn_port_update_sbrec()
> The copy of the patch that failed is found in:
>
>  /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --resolved".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
>
Hi Damijan,

If you put "ovn in the subject prefix like "PATCH ovn", then 0-day robot
will apply the patch to the ovn repo and it will
run the tests. Otherwise it will try to apply the patch to the ovs repo and
that's why you see this failure.
Thanks
Numan


>
> Please check this out.  If you feel there has been an error, please email
> aconole@bytheb.org
>
> Thanks,
> 0-day Robot
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Numan Siddique Aug. 6, 2019, 11:06 a.m. UTC | #4
On Fri, Aug 2, 2019 at 9:43 PM Numan Siddique <nusiddiq@redhat.com> wrote:

>
>
> On Fri, Aug 2, 2019 at 5:45 PM Damijan Skvarc <damjan.skvarc@gmail.com>
> wrote:
>
>> Memory leak happens because of redundand memory allocation for array
>> of single pointer. Issue was solved by removing this redundand allocation
>> and using address of pointer to created chassis sb_ha_entity instead.
>>
>> Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com>
>>
>
> Acked-by: Numan Siddique <nusiddiq@redhat.com>
>
>
I applied this patch to master. There was below checkpatch warning and I
fixed it like below before applying.
I have also submitted the patch for 2.12 branch -
https://patchwork.ozlabs.org/patch/1142719/

**********8
WARNING: Line is 94 characters long (recommended limit is 79)
#27 FILE: northd/ovn-northd.c:2509:
                        struct sbrec_ha_chassis *sb_ha_ch =
create_sb_ha_chassis(ctx, chassis,

Lines checked: 37, Warnings: 1, Errors: 0
********

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 2dbce4c16..e6953a405 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -2506,8 +2506,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
                     }

                     if (sb_ha_ch_grp->n_ha_chassis != 1) {
-                        struct sbrec_ha_chassis *sb_ha_ch =
create_sb_ha_chassis(ctx, chassis,
-                                                           chassis->name,
0);
+                        struct sbrec_ha_chassis *sb_ha_ch =
+                            create_sb_ha_chassis(ctx, chassis,
+                                                 chassis->name, 0);
                         sbrec_ha_chassis_group_set_ha_chassis(sb_ha_ch_grp,
                                                               &sb_ha_ch,
1);
                     }


Thanks
Numan


>
>
>> ---
>>  northd/ovn-northd.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>> index cd776fa..d99ae67 100644
>> --- a/northd/ovn-northd.c
>> +++ b/northd/ovn-northd.c
>> @@ -2506,12 +2506,10 @@ ovn_port_update_sbrec(struct northd_context *ctx,
>>                      }
>>
>>                      if (sb_ha_ch_grp->n_ha_chassis != 1) {
>> -                        struct sbrec_ha_chassis **sb_ha_ch =
>> -                            xcalloc(1, sizeof *sb_ha_ch);
>> -                        sb_ha_ch[0] = create_sb_ha_chassis(ctx, chassis,
>> +                        struct sbrec_ha_chassis *sb_ha_ch =
>> create_sb_ha_chassis(ctx, chassis,
>>
>> chassis->name, 0);
>>
>>  sbrec_ha_chassis_group_set_ha_chassis(sb_ha_ch_grp,
>> -                                                              sb_ha_ch,
>> 1);
>> +                                                              &sb_ha_ch,
>> 1);
>>                      }
>>                      sbrec_port_binding_set_ha_chassis_group(op->sb,
>>
>>  sb_ha_ch_grp);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>

Patch
diff mbox series

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index cd776fa..d99ae67 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -2506,12 +2506,10 @@  ovn_port_update_sbrec(struct northd_context *ctx,
                     }
 
                     if (sb_ha_ch_grp->n_ha_chassis != 1) {
-                        struct sbrec_ha_chassis **sb_ha_ch =
-                            xcalloc(1, sizeof *sb_ha_ch);
-                        sb_ha_ch[0] = create_sb_ha_chassis(ctx, chassis,
+                        struct sbrec_ha_chassis *sb_ha_ch = create_sb_ha_chassis(ctx, chassis,
                                                            chassis->name, 0);
                         sbrec_ha_chassis_group_set_ha_chassis(sb_ha_ch_grp,
-                                                              sb_ha_ch, 1);
+                                                              &sb_ha_ch, 1);
                     }
                     sbrec_port_binding_set_ha_chassis_group(op->sb,
                                                             sb_ha_ch_grp);