Re: [libvirt] [PATCH] Add test suite for viralloc APIs
On 04/09/2014 12:17 PM, Nehal J Wani wrote:
On Tue, Apr 8, 2014 at 8:18 PM, Daniel P. Berrange
<berrange(a)redhat.com> 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 :-)
>
> +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 ?
Hmm, that would probably be a reasonable enhancement to the testsuite,
if you'd like to contribute it as a followup patch.
> +
> + 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?
Yes, but it also doesn't hurt.
By the way, thanks for jumping in and reviewing; and you will get better
at it over time. I wish more new contributors would feel comfortable
giving a review; even a review that admits being uncomfortable as the
sole reviewer is still better than no comment at all.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)