[libvirt] [PATCH] Add test suite for viralloc APIs

Eric Blake eblake at redhat.com
Thu Apr 24 15:04:16 UTC 2014


On 04/09/2014 12:17 PM, Nehal J Wani wrote:
> On Tue, Apr 8, 2014 at 8:18 PM, Daniel P. Berrange <berrange at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140424/52932c9a/attachment-0001.sig>


More information about the libvir-list mailing list