[libvirt] [PATCH] Fix memory leak when lookup pool list with invalid type option

shyu shyu at redhat.com
Mon Feb 24 15:04:44 UTC 2014



----- Original Message -----
| From: "Martin Kletzander" <mkletzan at redhat.com>
| To: "shyu" <shyu at redhat.com>
| Cc: libvir-list at redhat.com
| Sent: Monday, February 24, 2014 4:48:01 PM
| Subject: Re: [libvirt] [PATCH] Fix memory leak when lookup pool list with invalid type option
| 
| On Mon, Feb 24, 2014 at 02:12:34PM +0800, shyu wrote:
| > There will be memory leak when lookup pool list with invalid type option
| > https://bugzilla.redhat.com/show_bug.cgi?id=1069068
| >
| > ==23060== Memcheck, a memory error detector
| > ==23060== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
| > ==23060== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
| > ==23060== Command: virsh pool-list --type invalid
| > ==23060==
| > error: Invalid pool type
| >
| > ==23060==
| > ==23060== HEAP SUMMARY:
| > ==23060==     in use at exit: 138,383 bytes in 1,559 blocks
| > ==23060==   total heap usage: 4,199 allocs, 2,640 frees, 21,372,544 bytes
| > allocated
| > ==23060==
| > ==23060== LEAK SUMMARY:
| > ==23060==    definitely lost: 8 bytes in 1 blocks
| > ==23060==    indirectly lost: 0 bytes in 0 blocks
| > ==23060==      possibly lost: 0 bytes in 0 blocks
| > ==23060==    still reachable: 138,375 bytes in 1,558 blocks
| > ==23060==         suppressed: 0 bytes in 0 blocks
| > ==23060== Rerun with --leak-check=full to see details of leaked memory
| > ==23060==
| > ==23060== For counts of detected and suppressed errors, rerun with: -v
| > ==23060== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 8 from 6)
| >
| 
| This info is useless since there is no information about the leak.
| You should've included this kind of info instead:
| 
| ==2472== 5 bytes in 1 blocks are definitely lost in loss record 5 of 84
| ==2472==    at 0x4A069EE: malloc (vg_replace_malloc.c:270)
| ==2472==    by 0x3B88281171: strdup (strdup.c:43)
| ==2472==    by 0x40EB40: _vshStrdup (virsh.c:129)
| ==2472==    by 0x40FC68: vshStringToArray (virsh.c:182)
| ==2472==    by 0x42A9F9: cmdPoolList (virsh-pool.c:857)
| ==2472==    by 0x40D240: vshCommandRun (virsh.c:1652)
| ==2472==    by 0x410B4A: main (virsh.c:3093)
| 
| > Signed-off-by: shyu <shyu at redhat.com>
| > ---
| >  tools/virsh-pool.c |    1 +
| >  1 files changed, 1 insertions(+), 0 deletions(-)
| >
| > diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
| > index 642b078..db4d3ae 100644
| > --- a/tools/virsh-pool.c
| > +++ b/tools/virsh-pool.c
| > @@ -859,6 +859,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd
| > ATTRIBUTE_UNUSED)
| >          for (i = 0; i < npoolTypes; i++) {
| >              if ((poolType = virStoragePoolTypeFromString(poolTypes[i])) <
| >              0) {
| >                  vshError(ctl, "%s", _("Invalid pool type"));
| > +                VIR_FREE(*poolTypes);
| 
| This does not properly free all the poolTypes in case this doesn't
| fails with i > 0.
| 
| >                  VIR_FREE(poolTypes);
| 
| And judging by the code, this is not upstream at all, right?  I guess
| this is not applicable upstream for half a year already, at least.
| Please submit patches that apply on top of current master when cloning
| from git://libvirt.org/libvirt.git .
| 

Hi Martin, I think I made an mistake. Since I found this bug with 0.10.2-29.el6_5.4 so I make 
this patch according v0.10.2-maint branch. I knew it was already be fixed on upstream. I thought
there should be fixed on v0.10.2-maint. Now I know that. 
Thanks for your kind remind.
 

| Thanks for your effort, but I believe this is already fixed within
| commit d64af6ce as I already commented in the Bug you referenced.
| 
| Martin
| 

-- 
Regards
shyu




More information about the libvir-list mailing list