| Message ID | 1564748041-12962-1-git-send-email-damjan.skvarc@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/1] ovn-northd: fixed memory leak in ovn_port_update_sbrec() | expand |
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
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 >
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 >
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 >> >
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);
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(-)