Re: [libvirt] [PATCH 2/5] tests: refactor virstoragetest for less stack space
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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)