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