[libvirt] [PATCH 2/5] tests: refactor virstoragetest for less stack space

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140404/60c74f3a/attachment-0001.sig>