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

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.

> +++ 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.

ACK.