[libvirt] PATCH: 15/28: Reduce return points for storage driver

[Daniel P. Berrange]

On Tue, Dec 02, 2008 at 03:02:17PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> > This patch reduces the number of return points in the storage driver
> > methods
> ...
> > diff --git a/src/storage_driver.c b/src/storage_driver.c
> ...
> > @@ -893,7 +924,7 @@ storagePoolListVolumes(virStoragePoolPtr
> >
> >   cleanup:
> >      for (n = 0 ; n < maxnames ; n++)
> > -        VIR_FREE(names[i]);
> > +        VIR_FREE(names[n]);
> >
> >      memset(names, 0, maxnames);
> >      return -1;
> 
> This might be worth putting in a separate bug-fix patch.
> At first I thought this was fixing a serious bug,
> but you can see that i is always smaller than maxnames,
> so the fix is just plugging a leak.
> 
> However, in looking at this I spotted a real problem:
> There are numerous statements like this:
> 
>     memset(names, 0, maxnames);
> 
> That zeros out only 1/4 or 1/8 of the memory it should.
> It should be doing this:
> 
>     memset(names, 0, maxnames * sizeof (*names));

memset() is horribly error prone - also have the classic of getting
the 2 & 3rd args the wrong way around. How about adding to memory.h
something like

  #define VIR_ZERO(buf, nelement) \
        memset(buf, 0, sizeof(*(buf)) * nelement)

also using your xalloc_oversized magic  ?


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|