On Tue, Dec 02, 2008 at 03:02:17PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange@xxxxxxxxxx> 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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list