On Tue, Apr 8, 2014 at 8:18 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> wrote: > In debugging a crash on OOM, I thought that the virInsert APIs > might be at fault, but couldn't isolate them as a cause. While > the viralloc APIs are used in many test suites, this is as a > side-effect, they are not directly tested :-) > > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> > --- > tests/Makefile.am | 5 + > tests/viralloctest.c | 387 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 392 insertions(+) > create mode 100644 tests/viralloctest.c > > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 122bf6c..c13cc15 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -140,6 +140,7 @@ test_programs = virshtest sockettest \ > viratomictest \ > utiltest shunloadtest \ > virtimetest viruritest virkeyfiletest \ > + viralloctest \ > virauthconfigtest \ > virbitmaptest \ > vircgrouptest \ > @@ -929,6 +930,10 @@ virkeyfiletest_SOURCES = \ > virkeyfiletest.c testutils.h testutils.c > virkeyfiletest_LDADD = $(LDADDS) > > +viralloctest_SOURCES = \ > + viralloctest.c testutils.h testutils.c > +viralloctest_LDADD = $(LDADDS) > + > virauthconfigtest_SOURCES = \ > virauthconfigtest.c testutils.h testutils.c > virauthconfigtest_LDADD = $(LDADDS) > diff --git a/tests/viralloctest.c b/tests/viralloctest.c > new file mode 100644 > index 0000000..abdd871 > --- /dev/null > +++ b/tests/viralloctest.c > @@ -0,0 +1,387 @@ > +/* > + * viralloctest.c: Test memory allocation APIs > + * > + * Copyright (C) 2014 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>. > + * > + */ > + > +#include <config.h> > + > +#include <viralloc.h> > + > +#include "testutils.h" > + > +#define VIR_FROM_THIS VIR_FROM_NONE > + > +typedef struct testDummyStruct { > + int a; > + int b; > +} testDummyStruct; > + > +static int > +testAllocScalar(const void *opaque ATTRIBUTE_UNUSED) > +{ > + testDummyStruct *t; > + int ret = -1; > + > + if (VIR_ALLOC(t) < 0) > + return -1; > + > + if (t == NULL) { > + fprintf(stderr, "Allocation succeeded by pointer is NULL\n"); > + goto cleanup; > + } Just out of curiosity, why don't we have this check after VIR_REALLOC_N, VIR_EXPAND_N, VIR_SHRINK_N and VIR_RESIZE_N ? > + > + if (t->a != 0 || > + t->b != 0) { > + fprintf(stderr, "Allocated ram was not zerod\n"); > + goto cleanup; > + } > + > + VIR_FREE(t); > + > + if (t != NULL) { > + fprintf(stderr, "Pointer is still set after free\n"); > + goto cleanup; > + } > + > + ret = 0; > + cleanup: > + VIR_FREE(t); > + return ret; > +} > + > + > +static int > +testAllocArray(const void *opaque ATTRIBUTE_UNUSED) > +{ > + testDummyStruct *t; > + size_t nt = 10, i; > + int ret = -1; > + > + if (VIR_ALLOC_N(t, nt) < 0) > + return -1; > + > + if (t == NULL) { > + fprintf(stderr, "Allocation succeeded by pointer is NULL\n"); > + goto cleanup; > + } > + > + for (i = 0; i < nt; i++) { > + if (t[i].a != 0 || > + t[i].b != 0) { > + fprintf(stderr, "Allocated ram block %zu was not zerod\n", i); > + goto cleanup; > + } > + } > + > + VIR_FREE(t); > + > + if (t != NULL) { > + fprintf(stderr, "Pointer is still set after free\n"); > + goto cleanup; > + } > + > + ret = 0; > + cleanup: > + VIR_FREE(t); > + return ret; > +} > + > + > +static int > +testReallocArray(const void *opaque ATTRIBUTE_UNUSED) > +{ > + testDummyStruct *t; > + size_t nt = 10, i; > + int ret = -1; > + > + if (VIR_ALLOC_N(t, nt) < 0) > + return -1; > + > + if (t == NULL) { > + fprintf(stderr, "Allocation succeeded by pointer is NULL\n"); > + goto cleanup; > + } > + > + for (i = 0; i < nt; i++) { > + t[i].a = 10; > + t[i].b = 20; > + } > + > + if (VIR_REALLOC_N(t, nt + 5) < 0) > + goto cleanup; > + > + for (i = 0; i < nt; i++) { > + if (t[i].a != 10 || > + t[i].b != 20) { > + fprintf(stderr, "Reallocated ram block %zu lost data\n", i); > + goto cleanup; > + } > + } > + > + if (VIR_REALLOC_N(t, nt) < 0) > + goto cleanup; > + > + for (i = 0; i < nt; i++) { > + if (t[i].a != 10 || > + t[i].b != 20) { > + fprintf(stderr, "Reallocated ram block %zu lost data\n", i); > + goto cleanup; > + } > + } > + > + if (VIR_REALLOC_N(t, nt - 5) < 0) > + goto cleanup; > + > + for (i = 0; i < (nt - 5); i++) { > + if (t[i].a != 10 || > + t[i].b != 20) { > + fprintf(stderr, "Reallocated ram block %zu lost data\n", i); > + goto cleanup; > + } > + } > + > + VIR_FREE(t); > + > + if (t != NULL) { > + fprintf(stderr, "Pointer is still set after free\n"); > + goto cleanup; > + } > + > + ret = 0; > + cleanup: > + VIR_FREE(t); > + return ret; > +} > + > + > +static int > +testExpandArray(const void *opaque ATTRIBUTE_UNUSED) > +{ > + testDummyStruct *t; > + size_t nt = 10, i; > + int ret = -1; > + > + if (VIR_ALLOC_N(t, nt) < 0) > + return -1; > + > + if (t == NULL) { > + fprintf(stderr, "Allocation succeeded by pointer is NULL\n"); > + goto cleanup; > + } > + > + for (i = 0; i < nt; i++) { > + t[i].a = 10; > + t[i].b = 20; > + } > + > + if (VIR_EXPAND_N(t, nt, 5) < 0) > + goto cleanup; > + > + for (i = 0; i < (nt - 5); i++) { > + if (t[i].a != 10 || > + t[i].b != 20) { > + fprintf(stderr, "Reallocated ram block %zu lost data\n", i); > + goto cleanup; > + } > + } > + > + for (i = (nt - 5); i < nt; i++) { > + if (t[i].a != 0 || > + t[i].b != 0) { > + fprintf(stderr, "New ram block %zu was not zerod\n", i); > + goto cleanup; > + } > + } > + > + VIR_SHRINK_N(t, nt, 5); > + > + for (i = 0; i < nt; i++) { > + if (t[i].a != 10 || > + t[i].b != 20) { > + fprintf(stderr, "Reallocated ram block %zu lost data\n", i); > + goto cleanup; > + } > + } > + > + VIR_SHRINK_N(t, nt, 5); > + > + for (i = 0; i < nt; i++) { > + if (t[i].a != 10 || > + t[i].b != 20) { > + fprintf(stderr, "Reallocated ram block %zu lost data\n", i); > + goto cleanup; > + } > + } > + > + VIR_FREE(t); > + > + if (t != NULL) { > + fprintf(stderr, "Pointer is still set after free\n"); > + goto cleanup; > + } > + > + ret = 0; > + cleanup: > + VIR_FREE(t); > + return ret; > +} > + > + > +static int > +testResizeArray(const void *opaque ATTRIBUTE_UNUSED) > +{ > + testDummyStruct *t; > + size_t nt = 10, at, i; > + int ret = -1; > + > + if (VIR_ALLOC_N(t, nt) < 0) > + return -1; > + > + at = nt; > + > + if (t == NULL) { > + fprintf(stderr, "Allocation succeeded by pointer is NULL\n"); > + goto cleanup; > + } > + > + for (i = 0; i < nt; i++) { > + t[i].a = 10; > + t[i].b = 20; > + } > + > + if (VIR_RESIZE_N(t, at, nt, 8) < 0) > + goto cleanup; > + > + if (at != 18) { > + fprintf(stderr, "Expected allocation of 16 not %zu\n", at); > + goto cleanup; > + } > + > + for (i = 0; i < at; i++) { > + if (i >= nt) { > + if (t[i].a != 0 || > + t[i].b != 0) { > + fprintf(stderr, "New ram block %zu was not zerod\n", i); > + goto cleanup; > + } > + } else { > + if (t[i].a != 10 || > + t[i].b != 20) { > + fprintf(stderr, "Reallocated ram block %zu lost data\n", i); > + goto cleanup; > + } > + } > + } > + > + VIR_FREE(t); > + > + if (t != NULL) { > + fprintf(stderr, "Pointer is still set after free\n"); > + goto cleanup; > + } > + > + ret = 0; > + cleanup: > + VIR_FREE(t); > + return ret; > +} > + > + > +static int > +testInsertArray(const void *opaque ATTRIBUTE_UNUSED) > +{ > + testDummyStruct **t; > + size_t nt = 10, i; > + int ret = -1; > + testDummyStruct *n = (void *)0xff; > + > + if (VIR_ALLOC_N(t, nt) < 0) > + return -1; > + > + if (t == NULL) { > + fprintf(stderr, "Allocation succeeded by pointer is NULL\n"); > + goto cleanup; > + } > + > + for (i = 0; i < nt; i++) > + t[i] = (void*)0x50; > + > + if (VIR_INSERT_ELEMENT(t, 3, nt, n) < 0) { > + if (nt != 10) { > + fprintf(stderr, "Expecting array size 10 after OOM not %zu\n", nt); > + goto cleanup; > + } > + goto cleanup; > + } > + Isn't the extra 'goto cleanup' in the inner block redundant, as we are already going to cleanup, because of OOM? > + if (nt != 11) { > + fprintf(stderr, "Expecting array size 11 not %zu\n", nt); > + goto cleanup; > + } > + > + if (n != NULL) { > + fprintf(stderr, "Expecting element to be set to NULL\n"); > + goto cleanup; > + } > + > + for (i = 0; i < nt; i++) { > + void *expect = i == 3 ? (void *)0xff : (void*)0x50; > + if (t[i] != expect) { > + fprintf(stderr, "Expecting %p at offset %zu not %p\n", > + expect, i, t[i]); > + goto cleanup; > + } > + } > + > + VIR_FREE(t); > + > + if (t != NULL) { > + fprintf(stderr, "Pointer is still set after free\n"); > + goto cleanup; > + } > + > + ret = 0; > + cleanup: > + VIR_FREE(t); > + return ret; > +} > + > + > +static int > +mymain(void) > +{ > + int ret = 0; > + > + if (virtTestRun("alloc scalar", testAllocScalar, NULL) < 0) > + ret = -1; > + if (virtTestRun("alloc array", testAllocArray, NULL) < 0) > + ret = -1; > + if (virtTestRun("realloc array", testReallocArray, NULL) < 0) > + ret = -1; > + if (virtTestRun("expand array", testExpandArray, NULL) < 0) > + ret = -1; > + if (virtTestRun("resize array", testResizeArray, NULL) < 0) > + ret = -1; > + if (virtTestRun("insert array", testInsertArray, NULL) < 0) > + ret = -1; > + > + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; > +} > + > +VIRT_TEST_MAIN(mymain) > -- > 1.9.0 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- Nehal J Wani -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list