On 04/09/2014 12:17 PM, Nehal J Wani wrote: > On Tue, Apr 8, 2014 at 8:18 PM, Daniel P. Berrange <[email protected]> > 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
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
