On 04/04/2014 02:54 AM, Peter Krempa wrote:
> On 04/04/14 06:32, Eric Blake wrote:
>> I'm about to add fields to virStorageFileMetadata, which means
>> also adding fields to the testFileData struct in virstoragetest.
>> Alas, adding even one pointer on an x86_64 machine gave me a
>> dreaded compiler error:
>>
>> virstoragetest.c:712:1: error: the frame size of 4208 bytes is larger than 4096 bytes [-Werror=frame-larger-than=]
>>
>> After some experimentation, I realized that each test was creating
>> yet another testChainData (which contains testFileData) on the stack;
>> forcing the reuse of one of these structures instead of creating a
>> fresh one each time drastically reduces the size requirements. While
>> at it, I also got rid of a lot of intermediate structs, with some
>> macro magic that lets me directly build up the destination chains
>> inline.
>
> Unfortunately it's not completely trivial to understand what's happening
> on the first glance.
I'll expand the commit message before pushing. Basically,
> -#define TEST_ONE_CHAIN(id, start, format, chain, flags) \
> +#define TEST_ONE_CHAIN(id, start, format, flags, ...) \
> do { \
> - struct testChainData data = { \
> - start, format, chain, ARRAY_CARDINALITY(chain), flags, \
the old code was populating data.files = chain, where 'chain' was an
intermediate variable and files is a testFileData*
> + size_t i; \
> + memset(&data, 0, sizeof(data)); \
> + data = (struct testChainData){ \
> + start, format, { __VA_ARGS__ }, 0, flags, \
> }; \
while the new code is populating data.files = { ptr, ptr, ... }, with no
intermediate chain variable, and where files is now an array of
testFileData*.
> + for (i = 0; i < ARRAY_CARDINALITY(data.files); i++) \
> + if (data.files[i]) \
> + data.nfiles++; \
Pre-patch, counting the number of files was done by the size of the
'chain' intermediate variable. Post-patch, it is done by counting the
number of non-NULL pointers in the 'files' array.
>
> + TEST_ONE_CHAIN(#id "a", relstart, format, flags1, \
> + VIR_FLATTEN_1(chain1)); \
Here, VIR_FLATTEN_1 is macro magic that takes a single macro argument in
the form '(a, b, c)' and turns it into multiple arguments 'a, b, c' to
TEST_ONE_CHAIN's use of '...'
> /* Raw image, whether with right format or no specified format */
> - const testFileData chain1[] = { raw };
> TEST_CHAIN(1, "raw", absraw, VIR_STORAGE_FILE_RAW,
> - chain1, EXP_PASS,
> - chain1, ALLOW_PROBE | EXP_PASS,
> - chain1, EXP_PASS,
> - chain1, ALLOW_PROBE | EXP_PASS);
> + (&raw), EXP_PASS,
> + (&raw), ALLOW_PROBE | EXP_PASS,
> + (&raw), EXP_PASS,
> + (&raw), ALLOW_PROBE | EXP_PASS);
Then for each test, I replaced uses of 'chain1' (the intermediate
variable)' with '(contents of chain1)' as a direct argument to TEST_CHAIN.
>
> As it's test code: ACK
Yes, we have that going for us - if it compiles and still passes the
testsuite, it was probably correct :)
--
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