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

[Patrick Mayweg <ma...@qint.de>]

Hi Jani,
thanks for pointing that out. I have made a new patch.
Regards,
Patrick

Log message:
[[[
Use the pool memory instead of stack memory for big buffers in all 
cases, which
do not need an interface change.

* subversion/libsvn_ra_dav/util.c
 (parse_spool_file) : use pool memory for big buffer.

* subversion/libsvn_repos/reporter.c
 (compare_files) : use pool memory for big buffers.

* subversion/libsvn_repos/delta.c
 (compare_files) : use pool memory for big buffers.

* subversion/libsvn_subr/stream.c
 (svn_stream_copy) : use pool memory for big buffer.
]]]

Jani Averbach wrote:

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