On Tue, Dec 02, 2008 at 03:02:17PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@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 :|