Re: [PATCH] Add test suite for viralloc APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]