[libvirt] [PATCH v2] tests: Lower stack usage below 4096 bytes

[Matthias Bolte]

2011/4/30 Eric Blake <eblake at redhat.com>:
> On 04/30/2011 05:10 AM, Matthias Bolte wrote:
>> Make virtTestLoadFile allocate the buffer to read the file into.
>>
>> Fix logic error in virtTestLoadFile, stop reading on the first empty line.
>>
>> Use virFileReadLimFD in virtTestCaptureProgramOutput to avoid manual
>> buffer handling.
>> ---
>>
>> v2: rebased to current git head.
>
> Just affects tests (other than the compiler warning, but since that
> doesn't trigger anywhere else in the code base), so this is safe for
> pushing before 0.9.1.
>
>>
>> -        # This should be < 256 really, but with PATH_MAX everywhere
>> -        # we have doom, even with 4096. In fact we have some functions
>> -        # with several PATH_MAX sized variables :-( We should kill off
>> -        # all PATH_MAX usage and then lower this limit
>> -        gl_WARN_ADD([-Wframe-larger-than=65700])
>> -        dnl gl_WARN_ADD([-Wframe-larger-than=4096])
>> +        # This should be < 256 really. Currently we're down to 4096,
>> +        # but using 1024 bytes sized buffers (mostly for virStrerror)
>> +        # stops us from going down further
>> +        gl_WARN_ADD([-Wframe-larger-than=4096])
>
> I'm actually happy that we're now down to 4096 - that's the size of a
> guard page on mingw (bigger than that, and you risk silent process
> termination instead of reasonable SIGSEGV on stack overflow).  Any
> smaller than that is a bonus, but you start to get into gnulib functions
> that use nearly 4k stack in a single function (such as gnulib's getcwd).
>  So I don't know if we still need to mention 256 as a goal in the
> comment, but I'm not heartbroken if you leave it as is.

We're actually down to ~2200 bytes max stack frame size at the moment.
Further reduction requires changing virStrerror usage to use smaller
buffers or move the buffers away from the stack. But the seems to be
no simple/nice solution for that so I stopped there :)

>> +++ b/tests/testutils.c
>> @@ -173,17 +173,16 @@ virtTestRun(const char *title, int nloops, int (*body)(const void *data), const
>>      return ret;
>>  }
>>
>> -/* Read FILE into buffer BUF of length BUFLEN.
>> -   Upon any failure, or if FILE appears to contain more than BUFLEN bytes,
>> -   diagnose it and return -1, but don't bother trying to preserve errno.
>> -   Otherwise, return the number of bytes copied into BUF. */
>> -int virtTestLoadFile(const char *file,
>> -                     char **buf,
>> -                     int buflen) {
>> +/* Allocate BUF to the size of FILE. Read FILE into buffer BUF.
>> +   Upon any failure, diagnose it and return -1, but don't bother trying
>> +   to preserve errno. Otherwise, return the number of bytes copied into BUF. */
>> +int
>> +virtTestLoadFile(const char *file, char **buf)
>> +{
>
> Looks like a nice rewrite of this function, and lots of fallout.

When I changed this function to allocate the buffer itself to filesize
+ 1 (instead of fixed 4kb) I first run into problem with it not
terminating anymore. Once I figured out that it got stuck in the while
loop with len == 0 I added the break statement on that condition and
it worked again. But I don't understand why it worked before with out
it. Maybe it's obvious for you and you can shed some light on it?

> ACK.
>

Thanks, pushed it, even if it's not a bug fix, but it doesn't touch
the actual library code.

Matthias