Re: [PATCH 1] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

On 2005-05-08 16:03+0200, Patrick Mayweg wrote:
> Hi,
> this patch removes all but one of the SVN_STREAM_CHUNK_SIZE buffers from
> the stack and use the pool instead. This fixes the stack overflow for
> the javahl binding on windows.

I have two comments:

> Index: subversion/libsvn_ra_dav/util.c
> ===================================================================
> --- subversion/libsvn_ra_dav/util.c (revision 14544)
> +++ subversion/libsvn_ra_dav/util.c (working copy)
> @@ -502,7 +502,7 @@
> {
> apr_file_t *spool_file;
> svn_stream_t *spool_stream;
> - char buf[SVN_STREAM_CHUNK_SIZE];
> + char *buf = apr_palloc(pool, SVN_STREAM_CHUNK_SIZE);
> apr_size_t len;
>
> SVN_ERR( svn_io_file_open(&spool_file, spool_file_name,

Later in the same function there is code like this:

      len = sizeof (buf);
      ...
      if (len != sizeof (buf))

This won't work with dynamically allocated buf.

> Index: subversion/libsvn_subr/stream.c
> ===================================================================
> --- subversion/libsvn_subr/stream.c (revision 14544)
> +++ subversion/libsvn_subr/stream.c (working copy)
> @@ -183,7 +183,7 @@
> svn_error_t *svn_stream_copy (svn_stream_t *from, svn_stream_t *to,
> apr_pool_t *pool)
> {
> - char buf[SVN_STREAM_CHUNK_SIZE];
> + char *buf = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE);
> apr_size_t len;
>

And same goes here:

  while (1)
    {
      len = sizeof (buf);
      ...

BR, Jani

-- 
Jani Averbach
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org