Message ID | 1435021348-9156-1-git-send-email-arei.gonglei@huawei.com |
---|---|
State | New |
Headers | show |
On (Tue) 23 Jun 2015 [09:02:28], arei.gonglei@huawei.com wrote: > From: Gonglei <arei.gonglei@huawei.com> > > Variable "r" going out of scope leaks the storage > it points to in line 3268. Interestingly Dave posted a different patch for the same issue. I prefer this approach. Reviewed-by: Amit Shah <amit.shah@redhat.com> Amit
<arei.gonglei@huawei.com> writes: > From: Gonglei <arei.gonglei@huawei.com> > > Variable "r" going out of scope leaks the storage > it points to in line 3268. > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > migration/rdma.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index cf5de7e..de80860 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -3262,12 +3262,13 @@ static const QEMUFileOps rdma_write_ops = { > > static void *qemu_fopen_rdma(RDMAContext *rdma, const char *mode) > { > - QEMUFileRDMA *r = g_malloc0(sizeof(QEMUFileRDMA)); > + QEMUFileRDMA *r = NULL; Dead initialization, please drop. > > if (qemu_file_mode_is_not_valid(mode)) { > return NULL; > } > > + r = g_malloc0(sizeof(QEMUFileRDMA)); > r->rdma = rdma; > > if (mode[0] == 'w') { Looks good otherwise.
On 23/06/2015 03:02, arei.gonglei@huawei.com wrote: > From: Gonglei <arei.gonglei@huawei.com> > > Variable "r" going out of scope leaks the storage > it points to in line 3268. > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > migration/rdma.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index cf5de7e..de80860 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -3262,12 +3262,13 @@ static const QEMUFileOps rdma_write_ops = { > > static void *qemu_fopen_rdma(RDMAContext *rdma, const char *mode) > { > - QEMUFileRDMA *r = g_malloc0(sizeof(QEMUFileRDMA)); > + QEMUFileRDMA *r = NULL; No need for the NULL initialization. Paolo > > if (qemu_file_mode_is_not_valid(mode)) { > return NULL; > } > > + r = g_malloc0(sizeof(QEMUFileRDMA)); > r->rdma = rdma; > > if (mode[0] == 'w') { >
Markus Armbruster <armbru@redhat.com> writes: > <arei.gonglei@huawei.com> writes: > >> From: Gonglei <arei.gonglei@huawei.com> >> >> Variable "r" going out of scope leaks the storage >> it points to in line 3268. >> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> --- >> migration/rdma.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/migration/rdma.c b/migration/rdma.c >> index cf5de7e..de80860 100644 >> --- a/migration/rdma.c >> +++ b/migration/rdma.c >> @@ -3262,12 +3262,13 @@ static const QEMUFileOps rdma_write_ops = { >> >> static void *qemu_fopen_rdma(RDMAContext *rdma, const char *mode) >> { >> - QEMUFileRDMA *r = g_malloc0(sizeof(QEMUFileRDMA)); >> + QEMUFileRDMA *r = NULL; > > Dead initialization, please drop. > >> >> if (qemu_file_mode_is_not_valid(mode)) { >> return NULL; >> } >> >> + r = g_malloc0(sizeof(QEMUFileRDMA)); >> r->rdma = rdma; >> >> if (mode[0] == 'w') { > > Looks good otherwise. Since you're touching this, you could r = g_new0(QEMUFileRDMA, 1) See commit 5839e53.
On 2015/6/23 17:20, Markus Armbruster wrote: > Markus Armbruster <armbru@redhat.com> writes: > >> <arei.gonglei@huawei.com> writes: >> >>> From: Gonglei <arei.gonglei@huawei.com> >>> >>> Variable "r" going out of scope leaks the storage >>> it points to in line 3268. >>> >>> Signed-off-by: Gonglei <arei.gonglei@huawei.com> >>> --- >>> migration/rdma.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/migration/rdma.c b/migration/rdma.c >>> index cf5de7e..de80860 100644 >>> --- a/migration/rdma.c >>> +++ b/migration/rdma.c >>> @@ -3262,12 +3262,13 @@ static const QEMUFileOps rdma_write_ops = { >>> >>> static void *qemu_fopen_rdma(RDMAContext *rdma, const char *mode) >>> { >>> - QEMUFileRDMA *r = g_malloc0(sizeof(QEMUFileRDMA)); >>> + QEMUFileRDMA *r = NULL; >> >> Dead initialization, please drop. >> >>> >>> if (qemu_file_mode_is_not_valid(mode)) { >>> return NULL; >>> } >>> >>> + r = g_malloc0(sizeof(QEMUFileRDMA)); >>> r->rdma = rdma; >>> >>> if (mode[0] == 'w') { >> >> Looks good otherwise. > > Since you're touching this, you could > > r = g_new0(QEMUFileRDMA, 1) > > See commit 5839e53. > I noticed this, but this whole file is using g_malloc, maybe using another patch fix them is better. This path is just fix memory leak. :) Regards, -Gonglei
diff --git a/migration/rdma.c b/migration/rdma.c index cf5de7e..de80860 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -3262,12 +3262,13 @@ static const QEMUFileOps rdma_write_ops = { static void *qemu_fopen_rdma(RDMAContext *rdma, const char *mode) { - QEMUFileRDMA *r = g_malloc0(sizeof(QEMUFileRDMA)); + QEMUFileRDMA *r = NULL; if (qemu_file_mode_is_not_valid(mode)) { return NULL; } + r = g_malloc0(sizeof(QEMUFileRDMA)); r->rdma = rdma; if (mode[0] == 'w') {