[libvirt] [PATCH] Fix misc OOM bugs
Mark McLoughlin
markmc at redhat.com
Thu Sep 3 11:07:33 UTC 2009
On Wed, 2009-09-02 at 15:11 +0100, Daniel P. Berrange wrote:
> * tests/testutils.c: Run test function twice, once to prime it for
> static allocations, once to count the non-static allocations.
> * tests/testutilsqemu.c: Initialize variable correctl
> * src/capabilities.c: Don't free machines variable upon failure
> since caller must do that
> * src/xm_internal.c: Add missing check for OOM in building VIF
> config param
> ---
> src/capabilities.c | 17 +++++++++--------
> src/xm_internal.c | 3 +++
> tests/testutils.c | 12 ++++++++----
> tests/testutilsqemu.c | 2 +-
> 4 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/src/capabilities.c b/src/capabilities.c
> index 193a9fe..289180d 100644
> --- a/src/capabilities.c
> +++ b/src/capabilities.c
> @@ -354,10 +354,6 @@ virCapabilitiesAddGuest(virCapsPtr caps,
> if (loader &&
> (guest->arch.defaultInfo.loader = strdup(loader)) == NULL)
> goto no_memory;
> - if (nmachines) {
> - guest->arch.defaultInfo.nmachines = nmachines;
> - guest->arch.defaultInfo.machines = machines;
> - }
>
> if (VIR_REALLOC_N(caps->guests,
> caps->nguests + 1) < 0)
> @@ -365,6 +361,11 @@ virCapabilitiesAddGuest(virCapsPtr caps,
> caps->guests[caps->nguests] = guest;
> caps->nguests++;
>
> + if (nmachines) {
> + guest->arch.defaultInfo.nmachines = nmachines;
> + guest->arch.defaultInfo.machines = machines;
> + }
> +
> return guest;
>
> no_memory:
> @@ -407,10 +408,6 @@ virCapabilitiesAddGuestDomain(virCapsGuestPtr guest,
> if (loader &&
> (dom->info.loader = strdup(loader)) == NULL)
> goto no_memory;
> - if (nmachines) {
> - dom->info.nmachines = nmachines;
> - dom->info.machines = machines;
> - }
>
> if (VIR_REALLOC_N(guest->arch.domains,
> guest->arch.ndomains + 1) < 0)
> @@ -418,6 +415,10 @@ virCapabilitiesAddGuestDomain(virCapsGuestPtr guest,
> guest->arch.domains[guest->arch.ndomains] = dom;
> guest->arch.ndomains++;
>
> + if (nmachines) {
> + dom->info.nmachines = nmachines;
> + dom->info.machines = machines;
> + }
ACK to both, good catch.
> diff --git a/src/xm_internal.c b/src/xm_internal.c
> index 71b852e..66f1e1c 100644
> --- a/src/xm_internal.c
> +++ b/src/xm_internal.c
> @@ -2048,6 +2048,9 @@ static int xenXMDomainConfigFormatNet(virConnectPtr conn,
> virBufferVSprintf(&buf, ",vifname=%s",
> net->ifname);
>
> + if (virBufferError(&buf))
> + goto cleanup;
> +
ACK
Check usages of xenDaemonFormatSxprChr() for a similar error
> diff --git a/tests/testutils.c b/tests/testutils.c
> index 7a1dbdc..5072fec 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -367,10 +367,7 @@ int virtTestMain(int argc,
> }
> }
>
> - if (testOOM)
> - virAllocTestInit();
> -
> - /* Run once to count allocs, and ensure it passes :-) */
> + /* Run once to prime any static allocations & ensure it passes */
> ret = (func)(argc, argv);
> if (ret != EXIT_SUCCESS)
> goto cleanup;
> @@ -385,6 +382,13 @@ int virtTestMain(int argc,
> testOOM++;
> virSetErrorFunc(NULL, virtTestErrorFuncQuiet);
>
> + virAllocTestInit();
> +
> + /* Run again to count allocs, and ensure it passes :-) */
> + ret = (func)(argc, argv);
> + if (ret != EXIT_SUCCESS)
> + goto cleanup;
> +
I don't follow this, maybe would be better as a separate patch with more
explanation. What is "prime any static allocations" ?
> approxAlloc = virAllocTestCount();
> testCounter++;
> if (testDebug)
> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index 58707e1..6b6a185 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -9,7 +9,7 @@ virCapsPtr testQemuCapsInit(void) {
> struct utsname utsname;
> virCapsPtr caps;
> virCapsGuestPtr guest;
> - virCapsGuestMachinePtr *machines;
> + virCapsGuestMachinePtr *machines = NULL;
This doesn't look like it's needed.
Cheers,
Mark.
More information about the libvir-list
mailing list