On 04/09/2014 12:17 PM, Nehal J Wani wrote:
> 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 :-)
>>
>> +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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list